firsttris / vscode-jest-runner

Simple way to run or debug one or more tests from context menu, codelens or command plalette
https://marketplace.visualstudio.com/items?itemName=firsttris.vscode-jest-runner
MIT License
264 stars 124 forks source link

add exactTestName option #282

Open janniks opened 1 year ago

janniks commented 1 year ago

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

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.

hnrchrdl commented 11 months ago

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 commented 11 months ago

@domsleee what do you think?

domsleee commented 11 months ago

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')

https://github.com/firsttris/vscode-jest-runner/blob/ad3d16a2a36debbc2a9679f9f43d7e378706a5e2/src/JestRunnerCodeLensProvider.ts#L47-L51

Thoughts?

firsttris commented 11 months ago

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 commented 3 weeks ago

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.