feat: expose is_test_file function to config

[AlexandrosAlexiou]

Useful in repos with peculiar test filenaming conventions

[thenbe]

Thanks for the PR!

As it stands, if a user defines is_test_file it will completely override the default one: https://github.com/thenbe/neotest-playwright/blob/c036fe39469e06ae70b63479b5bb2ce7d654beaf/src/discover.ts#L25-L47

That's largely a good thing as it gives users more tools to handle cases that we may not have accounted for (e.g. https://github.com/thenbe/neotest-playwright/pull/17). This can also alleviate some pain in more complicated setups like https://github.com/thenbe/neotest-playwright/issues/24.

However, I think we should mention, or link to, what the default implementation is, so that users wishing to extend it can do so from a sensible starting point.

The main point I'd like to communicate to users is that they should prefer the strictest pattern possible to avoid trying to parse unrelated files that may live in a test directory. To demonstrate the issue at hand, the current example would also try to parse jpg files that exist in the test directory (assuming the user has some screenshot tests).

[AlexandrosAlexiou]

Thanks for the PR!

As it stands, if a user defines is_test_file it will completely override the default one:

https://github.com/thenbe/neotest-playwright/blob/c036fe39469e06ae70b63479b5bb2ce7d654beaf/src/discover.ts#L25-L47

That's largely a good thing as it gives users more tools to handle cases that we may not have accounted for (e.g. #17). This can also alleviate some pain in more complicated setups like #24.

However, I think we should mention, or link to, what the default implementation is, so that users wishing to extend it can do so from a sensible starting point.

The main point I'd like to communicate to users is that they should prefer the strictest pattern possible to avoid trying to parse unrelated files that may live in a test directory. To demonstrate the issue at hand, the current example would also try to parse jpg files that exist in the test directory (assuming the user has some screenshot tests).

I agree, could you check this commit? I always also pass and call the default is_test_file to limit the files parsed and let the user define a stricter parsing function alongside the default. Let me know!

[thenbe]

I think I preferred your earlier approach, where we allowed users to completely override our implementation.

It's a pretty simple function and I don't expect us to change these patterns much over the years, so it's probably not too much trouble for users to just copy them into their config that one time in exchange for complete control over the logic.

[AlexandrosAlexiou]

I think I preferred your earlier approach, where we allowed users to completely override our implementation.

It's a pretty simple function and I don't expect us to change these patterns much over the years, so it's probably not too much trouble for users to just copy them into their config that one time in exchange for complete control over the logic.

Hmm I see your point, but everyone should have an extension logic there so it is like forcing the user to care for something that we care already. I would be a fan of letting him do the extra effort for his case and we take care of the extensions. That is why I went with this approach. But no strong opinion.

[thenbe]

The main reason I prefer the first approach is because I can see the second approach being an issue in the future, where someone asks for complete control over is_test_file (e.g. they want to use something like *.e2e.ts). Assuming we now take the second approach (where we also apply our filter on top of the user's filter), we'd be left in a tricky position. If we update is_test_file to not apply our filter, we'd break users who have already customized their is_test_file option, as they'd be forced to then also account for the default patterns. I'd prefer if we just ask them to do that now and get it over with.