Skip to content
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 exactTestName option #282

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

add exactTestName option #282

wants to merge 1 commit into from

Conversation

janniks
Copy link

@janniks janniks commented Oct 27, 2022

Background

Hi 👋 Power-user and big fan of the extension!

I often run into the issue of overlapping test names.
e.g. One test called "do something" and another called "do something else"; Running the first test using vscode-jest-runner will run both tests. This is because Jest's -t option regex matches. Most of the time I'd like to run the single exact test name. In the off chance I'm looking to run multiple tests, I could still go into the terminal and edit the command, but in 95% of cases I'm looking for one test and can be confused when an unexpected output is shown (until I realize multiple tests are running).

Changes

  • add the exactTestName boolean config option
    • which wraps the test name in ^ $ for an exact regex match
    • add option to package.json
    • add option to README.md
  • remove some unnecessary whitespace (my editor is set-up to do this automatically, lmk if I should re-add it)

FYI: I have verified that the wrapping works in my terminal, by manually replaying a generated terminal command from vscode-jest-runner and adding the characters — but, I haven't built the extension and tested end-to-end.

README.md Show resolved Hide resolved
@hnrchrdl
Copy link

can we merge this before the PR has its 1. birthday? :) would like to have this feature as well.

Most of the time I'd like to run the single exact test name.

Exactly, you should consider to make exact the default option.

@firsttris
Copy link
Owner

@domsleee what do you think?

@domsleee
Copy link
Collaborator

I think it's a good idea and can be made default at some point, but imo it should be an experimental flag for now.

I was testing it with the "Extension examples" build:
image

There's a problem with this example:

describe('nested', () => {
  describe('a', () => {
    it('b', () => {
      expect(true);
    });
  });
});

So if you click run on anything other than the inner-most block (b here), then it won't run any tests with the new setting enabled

For example, pressing a generates this command, which doesn't match any test (expected behaviour is to run test b):

node "node_modules/jest/bin/jest.js" "c:/Users/user/git/vscode-jest-runner/examples/examples.test.ts" -t "^nested a$"

So we could check that it is the inner most block during parsing in JestRunnerCodeLensProvider.

There is also this case to consider - note that the parser only sees the describe block here:

describe('tests', () => {
  generateTest('mytest');
});

function generateTest(name: string) {
  it(name, () => {
    expect(true);
  });
}

When you press run on the describe block, it should run tests mytest, but with exact matching, it won't match on ^tests$.

So what I'm thinking is - maybe the exact matching should only be on it blocks? Since the parser can't always determine what is the leaf node (second example), and it blocks can't be nested (jest enforces this)

It might be an idea to the parsedNode type of the CodeLens, and then retrieve it later (the condition would be parsedNode.type === 'it')

const fullTestName = escapeRegExp(findFullTestName(parsedNode.start.line, parseResults));
codeLens.push(...codeLensOptions.map((option) => getCodeLensForOption(range, option, fullTestName)));
return codeLens;

Thoughts?

@firsttris
Copy link
Owner

my thoughts:

genereally the test name hierarchy for the regex can be quite specific?
-t "describeName TestName NestedTestName"
means you can easily structure your tests that it matches just one Test.

Indroducing this option will possibly introduce new issues as @domsleee just showed.
Someone will enable this option and in return complain why it doesnt run his tests?
Maybe even more people will complain then requesting this feature?
Remember every project created with nx-tools has this addon installed.
Means im really scared of breaking something.

Maybe enabling it only for the "it" block will solve it? but still you have the problem with nested it's..

We should not merge this, till we found a solution for this questions.

best regards
tristan

@donaldpipowitch
Copy link

I was about to create pretty much the same MR right now. Can we merge this by renaming exactTestName to experimentalExactTestName or something similar and leaving a note that this might not work for nested test functions? We have a big test landscape and pretty much only use flat tests functions and this would really help us a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants