Closed mindplay-dk closed 3 years ago
I've never used test framework other than jest. I think our tests are running well now, and it may not be better to change the framework, so I think this may be able to pause?
I had a hard time understanding our test code, and I think most of our problems have been solved. We can complete the rest of the test cases.
Is tape more popular than jest?
If so, is it possible for us to use it in fre2?
Well, Jest is by far the most popular of those two - but I've seen many people asking/commenting on the same issues we've run into, and the situation doesn't seem to be improving.
I suspect that Jest is mostly popular because it's the de-facto standard test-framework for anyone who uses React - they have facilities specifically designed for that, so... many React users -> many Jest users.
My main concern about Jest is, I don't feel like I can trust it - I don't even know if it's running all the tests or hitting all the assertions... things often look fine, and then I discover down the line that I forgot an await
statement in a test or something and half the test wasn't running at all.
I don't know if all our tests are actually passing for the right reasons - as you saw, it's completely possible to write a test with a loop full of assertions that Jest just ignores because you called done
.
From what I can tell, tape
is much more reliable in that way at least.
Our test code wouldn't really change much at all - tests are still just functions being passed to a test
function, assertions may have slightly different names but basically work the same, and we're only using one or two of them.
Anyhow, let's leave this here for now and think about it...
I'm sorry for using done()
by mistake. Yes, as you said, sometimes I don't know why the test passed. I think we need a proper time to change the test framework.
In fact, I have little experience in testing. Thank you very much for your guidance and help.
I think we can open this issue and come back after settling for a while……
hey, I think we can continue this issue now, thank you for your proposal.
tape is simple and cute, if the work is too heavy, we can migrate in stages.
Okay, so I think the first step is just to get the change from #117 ported over to tape
and see if that changes anything.
If that doesn't help, we'll know the problem isn't Jest (which could mean it's JS-DOM or something else we haven't thought of) and that'll help us decide whether to proceed with tape
or what to do next.
Ok 😉
Man, it's worse than I thought...
I was using jsdom
, which is what jest
uses, but I was using another package jsdom-global
to expose (and clean up) all the DOM browser globals - and it turns out performance
, which we need for scheduling, isn't supported, and jsdom-global
is most likely a dead project anyhow, so...
I dug into jest
, which has it's own implementation of a browser environment based on jsdom
, but this was purpose-built for jest
and isn't really portable.
I also poked around in Jest's fake implementation of timers to answer another question: what happens to timers that run too late? It turns out, these get destroyed and won't finish executing if the JS-DOM instance is cleaned up.
This explains why a line like this one actually executes, even though it's being run after the JS-DOM instances has been cleaned up: the fake timers (if they're even fake?) are an entirely different implementation than the one in Jest.
These differences could give us even more problems rather than solving them.
At this point, I'm sort of fed up with jsdom
and I don't believe it's worth the effort - I think we have to figure out how to get a proper headless browser environment running for the tests... jsdom
and jest
are likely too unrealistic on some of the important points around scheduling - and even if we could get this to work, as mentioned, it doesn't prove that this works in a real browser.
We could still use tape
, but maybe at this point it's better to look at something more mainstream like mocha
and karma
, which is what jest
is based on anyhow - so our tests wouldn't need to change much, and we can probably reference an existing project for many of the solutions, for example the package.json
of preact
, as they're already solving all of the problems we need to solve. (JSX, ES6, headless browser, etc.)
I think maybe jest
is a good choice if you're building with a standard frameworks (React, Angular, VueJS) but at this point I'm not convinced it's a good choice for testing a UI framework - I think mocha
itself is probably a better choice for something more low-level, and at least Angular, Vue and Preact all use karma
for the browser stuff.
This is turning out to be quite the adventure, ha ha. 😂
If only performance is not supported, then polyfills can be found, and I don't think js-dom is a good choice.
We can refer to the practice of preact, multiple libraries are used at the same time.
Anyway, I don't think jest is a good choice for testing js framework now.
I've gotten one test ported to mocha
, and it passes - changes from jest
test-format are pretty minor, but the built-in assertions provided by node
were lacking, so I also added chai
, which provides nice assertions. I could easily port all the tests, but this first test doesn't have any browser-dependency - there's no point in porting everything until we've got a working browser environment integration.
So I've attempted to create a repeatable headless Chromium setup with mocha
. I have the integration working - it's attempting to launch Chromium, but currently fails because of a missing dependency.
I've also tried with the puppeteer
package, and it fails just the same - and people seem to accept the solution that all dependencies have to be manually installed, so I don't know... maybe there's no such thing as an npm
installable Chrome environment?
I don't think preact
have that either, so maybe that's just the sort of pain that the whole JS community puts up with and solves on every individual system? 🤷♂️
I don't think our test needs to be so troublesome. It doesn't need headless browser(such as puppeteer), but only Dom and BOM. May be like this:
import undom from 'undom'
import performance from 'performance-polyfill'
We only need a small Dom and BOM to simulate the variables used by fre, such as document
、requestAnimationFrame
、performance.now()
The difference between now and js-dom
is that we can use a third-party library or our own code to implement these APIs.
I wouldn't trust undom
or polyfills anymore than I would trust jsdom
- the net result of putting together a mock DOM and some polyfills is going to be much the same as jdsom
, and it's never going to behave exactly like a real browser in the ways that matter, not for reliable testing of scheduling etc.
It's the same problem as with jsdom
- even if you can get it working and get the tests passing, what does that prove? The polyfill for MessageChannel
for example can't be implemented in a way where it executes like it does in a real browser.
Frameworks really need to be tested against a real browser.
At the very least, I think we need an option to run the tests on a local browser during development - even if we have to run them with polyfills on the CI server, at least someone can do a real browser-based test on their local system before pushing a change.
It is hard to test the scheduler, not only fre, but also react, But I think this is not about headless browser. we just have not found a way to test it in node.
At the very least, I think we need an option to run the tests on a local browser during development
Maybe devtools or this
Well, finally some good news!
I scrapped jsdom-global
and manually set up jsdom
instead - and got the first test to pass! 🥳
https://github.com/mindplay-dk/fre/commit/ac7e5d9289423f44e715521b7443f61cc78451ac
More good news: this is the reconciliation test that we couldn't get to work with jest
, and it's passing with tape
now, with stateNumber >= 1
as it was meant to be! I even shaved a few bytes by removing your stand-in for requestAnimationFrame
and adding a trivial mock to the test-setup instead.
So it looks like there's a way forward with tape
after all.
I'll try to port the rest of the test-suite one of the coming days. 🙂
I will say again though: a passing test with mocks for requestAnimationFrame
and others does not guarantee the same test would pass in a browser - mocks and stubs under node can never behave exactly like a real browser, so I still think we should add an option to run the test in a local browser at some point.
For now, I'm happy get rid of jest
and get a faster, working test-suite though! 👍
@mindplay-dk Wow great, look forward to it!
I will say again though: a passing test with mocks for requestAnimationFrame and others does not guarantee the same test would pass in a browser.
I see, so we can only test DOM in node, and then test scheduler through chrome panel in the future.
I see, so we can only test DOM in node, and then test scheduler through chrome panel in the future.
Well, it'll run the full suite of tests - just that those tests will be effectively testing the integration with a bunch of mocks and stubs, whereas what we're really interested in is integration with a browser.
But we'll figure out later how to run the tests without the mocks and stubs. 🙂
Well, I'm stuck again. 😣
The problem is once again testUpdates
and the timing issues I've mentioned many times now - there just isn't any reliable way to know when the scheduler is finished, so I'm stuck with update.test.jsx
again, and I really don't think there's any way to make this properly testable right now.
To explain the problem, I've isolated a very simple test on a branch.
I've put a bunch of log statements in testUpdates
, so you can see the order in which things execute.
AWAIT 0
SET CONTENT 0 { type: [Function: Component], props: {}, key: null, ref: null } [ Text { last: null } ]
NOOP EFFECT
RUN TEST 0
✔ should be equal
✔ should be equal
RESOLVE 0
RESOLVED 0
AWAIT 1
SET CONTENT 1 { type: [Function: Component], props: {}, key: null, ref: null } [ HTMLDivElement {
last:
{ type: 'text',
props: [Object],
op: 3,
tag: 0,
parent: [Object],
sibling: null,
parentNode: [Circular],
node: [Text],
insertPoint: null,
pendingProps: [Object] } } ]
✖ test exited without ending
-----------------------------
As you can see, the test somehow exits before effect
is dispatched - we see SET CONTENT 1
, but never RESOLVED 1
, and we never see RETURN
, so we never reach the end of testUpdates
at all.
The update-test itself says await testUpdates(...)
, so the test shouldn't end until that promise resolves... but we never even reach the TEST ENDS
log statement before the script somehow exits.
Maybe this tape-promise
package doesn't really work? It seems to work in other cases though - the reconciliation test also does a bunch of updates, and it works fine.
I've spent more than half a day trying to solve it - I have no ideas. 😐
How should I debug your code? Without debugging, it seems that it is because of content
It shoud like this:
await testUpdates([
{
<Compoent value={[]}/>, //give it a props to rerender
test: ([button]) => {
is.equal(+button.textContent, 0)
is.equal(updates, 1)
// trigger several functional state updates:
// button.click()
// button.click()
// button.click()
}
}
I want to debug the code. Is there any way to pull your branche?
it seems that it is because of
content
In this particular case, the component doesn't need to be updated by testUpdates
- the setState
calls inside the onClick
handler (commented out for now) will trigger the updates.
But first problem is to get the test to actually run at all - before the script exits.
Is there any way to pull your branche?
You want to add a remote, then check out (not pull) my branch.
You were right - I finally got that test to pass, I just had to use the <Component value={[]}/>
hack to bypass the optimization, as you suggested.
Will post an update soon.
@mindplay-dk In fact, another optimization is hit here. When the state does not change, the component will not be updated.
const content = <Compoent />
setContent(content)
setContent(content) // do not rerender
It should be like this:
setContent(<Component />)
setContent(<Component />)
For the record, a lot of the test issues you've been closing, those tests don't actually work - many of the assertions aren't being run at all... tape
doesn't report any errors if you make assertions after the test has ended - those assertions are just ignored.
I had started porting the tests to another framework called zora
, after discovering how poorly this works in both jest
and tape
, but my branch is now 40+ commits behind and probably needs loads of manual merging to get it up to date.
And there are still problems with that branch - for one, it suffers from the same problem as current tests: some tests will pass when run individually, but they will fail when you run them together, due to race conditions. zora
provides a solution for this, but it's more work.
Sorry, but I can't keep up - too many things are changing daily, and I can't port and fix the entire test-suite fast enough to finish before my work is already outdated and can't be merged.
I'm going to take a break as I'm wasting too much time on work I can't finish...
@mindplay-dk Thank you very much, don't rush, take your time. I recently checked that ,I'm sick and I need to be hospitalized. treatment is needed and I need to paused few months. Let's wait together.
Sorry to hear that! I hope you get well soon. 🌻
Just to mention that this is written in Typescript and tape doesn't like Typescript at all. The devs don't want to support it or have anything to do with it from what i've seen in the issues. That might be an issue at some point too.
Jest is bigger but well adapted for UI library like React, Fre, etc
Also for adoption, makes sense to use the same tool as most of the UI library similar to Fre
Just giving my 2 cents here :)
I love tape
and use it as much as i can but it definitely has issues when it comes to Typescript and UI library(more setup to make like @mindplay-dk said).
@wcastand
https://www.npmjs.com/package/@web/test-runner
Do you know test-runner? This is a popular testing tool recently, it supports headless browsers.
The reason fre doesn't want to use jest is because jest cannot accurately simulate browser behavior, which makes our testing more difficult, especially time slicing and tearing.
We need to use a headless browser to ensure accurate behavior. I hope I can switch to test-runner and try it together?
A lot of people use Cypress to test browser behavior, is it what you look for?
Took a quick look at test-runner, seems different than Cypress. Also look pretty nice, using esbuild
or buildless setup with ESM which is cool. Worth looking into it for sure :)
I don't know test-runner, i'm no expert in testing framework, especially browser ones like cypress or test-runner. I alway find them super heavy and too time consuming for not much return (talking from a agency/product company point of view, for library like fre
it's probably different).
I alway find them super heavy and too time consuming for not much return.
Yes they are very heavy. The test task is given to CI, and the best result is that developers need not to install huge dependencies.
In order to test the features of concurrent mode, we have to do this, because fre rendering is asynchronous, it needs more test case accuracy than synchronous.
Let me try it next, I hope such a heavy thing can bring benefits.
When i talk about heavy, i'm talking about the time it takes to get them to work and maintained for the dev. Size is not really an issue when it comes to dev tools since it's mostly run on CI like you said.
But in my previous company, they had cypress and maintaining that took more time than making features lol (not even talking about the time it took to run the tests on CI, half the time was cypress tests even in async)
But for fre
it's necessary i'd say and will probably be easier to maintain since the API is smaller.
I refactor the algorithm again, and then used a new technique called centralized paints
This is a technology similar to DocumentFragment
. It operates DOM in memory and finally paints to the screen at final time.
Now fre is super fast, even faster than vanilla js.
What do you think about switching from
jest
totape
?I've got a preliminary setup here:
https://github.com/yisar/fre/compare/master...mindplay-dk:use-tape
This is unfinished -
tape
is much simpler thanjest
, but it doesn't come with "batteries included", so it takes more work to set this up.For now, I have Babel with JSX and
async
support working - this seems already much safer, because I can clean up JS-DOM after each test, which means that timeouts get flushed, which meanstape
can catch assertions that run after the promise resolves. (the commented-out lines demonstrate this.)I still need to set up code coverage and XML reporting for CI integration - before I can port the actual tests, so a lot of work left.
Do you think I should continue with this?