-
Notifications
You must be signed in to change notification settings - Fork 627
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add test_suite module to deno_std to help bridge a gap in testing experience between Deno and Node #1991
Comments
I'm in favor of this proposal. I recently ported many npm modules from crypto-browserify to std, and converting all (links to past similar discussions: #1389, #931, #1831) Regarding the past discussions/objections, maybe let's not try to make it compatible to mocha/jasmine/jest, but just make it 'enough similar' to them. How about include this as |
It's close enough that it should be easy to adapt existing mocha, jest, and jasmine tests to using describe and it from these functions. I believe in most cases it would be as simple as adding the import at the top. For hooks you may need to rename before to beforeAll and same for after, depending on which test runner you were using. I went with including all in that hook name to make it more clear that it's not beforeEach. I'm not sure about the other test libraries but currently mine has a limitation of calling the before and after hooks once per describe and at the top level. It also requires them to be called at the top of the describe block so that it is more clear the hooks apply to all subtests. It would be easy to adapt to allow hooks to be registered after tests are in a describe block. If would also be easy to change it to allow you to call the hook functions multiple times per describe block. One issue I currently have is with nested only flag, I can't modify the top level test because it registers immediately, currently if you add the only flag to a nested test, it will omit all nested test cases within the top level test that do not have to the only flag. I couldn't retroactively add only to top level tests that had already been registered with Deno.test. If there was a way to trigger registering at the end right before tests start running, I could resolve the issue. |
The fact that the describe and it definitions extend Deno.TestDefinition and has all the same call signatures minus the fn being different makes it so that it would be easy to switch from using Deno.test and t.step to using describe and it. |
One difference between this module and the node test frameworks is that this one also supports a flat style, an example of that can be found in the readme. What it means is that describe does not have to take a fn callback and will return an object that can be used to add tests to the group. The it and describe functions optionally take that suite object as a first argument or it can be included in it's definition to signal it belongs to the describe test block. The motivation for having the flat style is to make it easy to have deeper subgroups without the lines getting to long. With deno fmt line limit, it can make the dealer tests more difficult to read due to them having to be broken apart across more lines due to the amount of indentation they have. |
If I could get an example of a test case that would be acceptable without using mocking, I could get started on re-writing the tests so that it will be easier to add this module to deno_std later on. I'll wait to do the actual work until it's agreed on that it should be added or in what way it should be added. I've created a few issues in the test_suite module's github repository regarding improving the module and preparing it for being able to be added to deno_std. |
I've cleaned up the TestSuite interface, hiding all the functions and properties that are used internally only. The TestSuite object now just has a symbol on it for looking up the internal representation for a group of tests. I've removed the third party dependency on Vector that was mentioned in my original post. |
Nice!
👍
How much do you expect that re-write would take amount of time? If it takes a lot of time, then I guess including the entire |
It depends on what the new tests would need to look like, I'd need an example to estimate re-wrote time. Also I mostly just do work on side projects like this on weekends because I work as a software engineer full time during the week. The mock module is similar to Sinon.js but I wrote it from scratch in TypeScript. I'm not sure how much work it would take to get it into a state where it would be accepted for being added to deno_std. It also currently depends on my third party collections module like test_suite previously did. Changing it from using Vector to Array wouldn't be hard. But it also uses a Red-Black tree I wrote for accurately copying the behavior of Deno's timers in mocks FakeTime. Deno has or had a Red-Black Tree in it's JavaScript code for internal use only at one point. https://deno.land/x/[email protected]/trees/rb_tree.ts If Deno had a Red-Black Tree in it's std modules it would make it easier to adapt mock to no longer having third party dependencies. I think some of the data structures I implemented in my collections module would be good to add to std/collections. JavaScript doesn't have a double ended queue, binary heap, or red black tree. Other languages like Rust and Java have standard double ended queues and binary heaps. I'm not sure if any other languages have standard red black trees though. JavaScript`s Array can serve as a double ended queue because it has shift and unshift functions, but they are not efficient to use because every call triggers reindexing the array. My collections module has 98% test coverage, it's just missing a few functions on the Vector class that I would like it to have but just haven't gotten around to doing due to having other higher priority projects I'd like to work on. |
I feel like re-writing the tests in the way @bartlomieju described would probably make this require much more test coverage code than it currently has because we would probably want to also verify all of the different The current test coverage just verified that properties like |
@KyleJune sorry for late response. Let me try to address as many details as possible.
I think we could consider adding
Correct, our implementation of RB tree is internal and can't be exposed. I'm also open to adding this module to
We can do this a multi-step process. There were requests to add snapshot testing utility to
Is this something that could be changed to preserve behavior of Node's testing framework with multi-level grouping? The fewer changes users migrating would have to apply to better. There's also a quick and dirty polyfill written by @lucacasonato some time ago (https://gist.github.com/lucacasonato/54c03bb267074aaa9b32415dbfb25522). |
Also ref #1389 |
I could work on adding that after adding the data structures it uses to std/collections.
I'll try getting some PRs up this weekend for that if I have time to. Then after getting it and mock added, do the work to add test_suite.
I'm not too familiar with snapshot testing. If that capability is added and I'm given an example of how to use it, I could re-write the tests to use it for verifying the tests get registered correctly instead of using mocking.
It's compatible with the node testing framework behavior for multi-level grouping. The difference is basically just that describe will return a reference to the test group that can be passed into other other describe/it function calls to make them children of it. The flat and non flat style of multi-level grouping can be mix and matched. Both styles work and examples of each can be found in the readme. I believe with the flat style being available, it's easier to start organizing tests by just creating a group with The flat style is basically an extra feature that I could strip out if needed to be able to get it into deno_std. I personally prefer using the flat style for my own tests. |
Sounds good! Let me know if you need any help.
It's essentially a way of asserting that output matches what is expected with some wildcards. Examples: We use this technique heavily in
Perfect, I guess I misunderstood. |
Oh nice, I get what you mean now. Still with switching to that style, we would probably want to add test coverage that |
I've removed the test dependency on the mock module's FakeTime class. That was the one that used the collection module's Red-Black Tree. I'm working on a refactor of the mock module then will make a PR for adding spy and stub to std/testing. Those are all that are needed for the test_suite module's test coverage. I think FakeTime would be good to add to std/testing too but that could wait until after getting the basic spy/stub functions and describe/it functions into std/testing first. |
I resolved the issue of not being able to have multiple of the same type of a test hook globally or with a describe block. I'll try to create a PR to add test_suite to std next weekend. It will branch off of my branch for adding mocking utilities since the test coverage uses them. |
Discussed in https://github.com/denoland/deno_std/discussions/1942
Originally posted by KyleJune February 20, 2022
In the discord, @bartlomieju suggested opening an issue here about possibly adding my third party module to deno_std to help bridge a gap in testing experience between Deno and Node.
The test_suite module is not a direct port of Jasmine, Jest, or Mocha but has similar functionality. The
describe
andit
functions take a test definition that is an extension of Deno.TestDefinition. For thedescribe
andit
functions, I copied all the call signatures Deno has forDeno.test
andt.step
.I implemented the
only
option for nested tests which you currently cannot do with thet.step
.It depends on 1 third party module I wrote called collections. It just uses the
Vector
class from there instead ofArray
to make it easier to remove steps when a new test step is focused. It wouldn't be difficult to re-write the parts that useVector
to not use it.The test coverage depends on another third party module I wrote called mock. It is not a direct port of sinon.js but is similar. I wrote it from scratch in TypeScript for Deno because when I originally started using Deno, I wasn't able to get sinon.js to work.
I have 92.90% test coverage currently. For getting rid of the third party module dependency, they would need to be re-written. Currently the testing works by mocking
Deno.test
and creating a fakeTestContext
. @bartlomieju said he thinks this could be verified by integration tests - spawning a subprocesses and checking if output matches instead of using mocking.More information and examples can be found in the readme here.
The interface is documented using Deno Doc.
The text was updated successfully, but these errors were encountered: