-
Notifications
You must be signed in to change notification settings - Fork 80
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
Adds support for parameterized tests #51
Adds support for parameterized tests #51
Conversation
…en checking if test comes from parameterized test beacuse of old jest version location bugs
…inal # Conflicts: # lua/neotest-jest/init.lua
Saw you were blocked by the neotest PR, so got it merged 😄 Very cool to see this! |
I think that the PR is ready for testing. If someone has the time to checkout to this branch and report any problem I would be grateful. |
I'm going to test this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update your description, because the referenced blocking PR on neotest is merged, as @rcarriga stated.
adapters = { | ||
require('neotest-jest')({ | ||
..., | ||
jest_test_discovery = false, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jest_test_discovery = false, | |
jest_test_discovery = true, |
Did you mean this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
acutally I meant to use false
here, because if someone does not diable discovery
key and automatically will update the plugin, this option will run jest commands on all of the files in the project. I think that this solution should be opt-in
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but wouldn't it be misleading if the e.g. for enabling the discovery displayed the value as false?
I have tested this a bit. Works very well. I really like the idea of resorting back to jest to discover the tests. This saves reimplementing all the logic. Do you think this is mergable? |
I think after resolving conflicts its mergable. I'll try to resolve them today |
I think I found some bugs. Consider the following test: describe.each([null, undefined])('thing', (value) => {
test('is null', () => {
expect(value).toBeNull();
});
test('is undefined', () => {
expect(value).toBeUndefined();
});
test('is not truthy', () => {
expect(value).not.toBeTruthy();
});
});
describe('thing', () => {
const value = null;
test(`is null`, () => {
expect(value).toBeNull();
});
test('is undefined', () => {
expect(value).toBeUndefined();
});
test('is not truthy', () => {
expect(value).not.toBeTruthy();
});
}); Now try running only the first test (with the backticks in the testname). It does not work, this test does not get a success/failure symbol, instead the whole describe block is being run. |
So about the first scenario -> this is a limitation (or not) of the plugin. If you have
About the second scenario: its the problem on master branch #46 |
Then I'm fine with merging it :) |
I use this for months now and it seems usable and stable. Any option of merging and/or reviewing it @haydenmeade ? |
Hey!
This is a PR that allows to see and run parameterized tests in the summary window.
The main algorithm here is:
jest
to discover every test in the filejest
that are on the same position returned by treesitterCloses: #2
How to run:
Checkout
neotest-jest
to my branchDemo:
Screen.Recording.2022-12-27.at.20.29.03.mov
TODO:
test.each
andit.each
add support for(maybe in different PR)describe.each
run_nearest
jest skips them