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

Switch between file and line number #65

Open
a8t opened this issue Dec 19, 2022 · 6 comments
Open

Switch between file and line number #65

a8t opened this issue Dec 19, 2022 · 6 comments

Comments

@a8t
Copy link

a8t commented Dec 19, 2022

Hi, thanks for the library! I think this is a feature request, but with pointers I'd be happy to look into implementing it.

I often want to switch between running all tests in a file and running test at a given line. Is there an easy way to achieve this? I think it'd only make sense if the currently running tests are all in one file.

@randycoulman
Copy link
Owner

@a8t You can use the p command with either the filename (or a unique substring of the filename) OR with the filename:linenumber syntax that mix test supports.

For example, if you have test file names test/foo_test.exs and test/bar_test.exs, you can use p foo to run all of the tests in test/foo_test.exs. To specifically run the test at line 42 in test/foo_test.exs, you'd have to use the full filename plus line number as p test/foo_test.exs:42 instead.

@byronalley
Copy link
Contributor

I'd also find this very useful -- eg. if you're only running one test, to be able to type eg: l 42 and have it run only the test on line 42. I could also implement this.

@randycoulman
Copy link
Owner

@byronalley How would you see this potential new command working if there are more than one file selected? I'm a bit reluctant to add commands that only work in limited circumstances or that have to report errors due to disallowed use.

As I said above, it is possible to provide a file:line pattern already, but I've been thinking about ways to make it quicker to run a single file/line, so I'm curious to hear how you think this might work with a new command.

One option I've thought of is to allow a substring:line pattern, and if the pattern only matches a single file, then append the line number to it, but again, that raises the possibility of either ignoring the line number or reporting an error.

In Elixir 1.16, mix test now allows multiple file:line patterns. We could append the line number to all matching patterns, but it's highly unlikely that we'd want to run exactly the same line in every matching file, so that doesn't seem very useful.

@byronalley
Copy link
Contributor

Thanks @randycoulman -- I definitely appreciate that reluctance, and fwiw mix_test_interactive is already super useful for me--thanks! In my experience it's just such a common use case that it seemed worth it -- I find when I'm developing, I'm more often just working on one module gradually adding new tests via TDD, or maybe going through a module with failing tests, fixing code one test at a time. So currently I end up having to quit the test runner and restart with a different file and line number, or just run all the tests in the file.

Since adding a line number only makes sense per file, I'd thought of it as an additional filter that only applies if there's a single test being run. So to summarize I believe the options are:

  1. It's a) ignored or b) a short error is displayed if you try to use it on multiple files or maybe on a pattern match at all. Probably it's only allowed on the file:line combo.
  2. It only applies to the first matching file in a list. This seems a bit weird but it would work.
  3. There could be syntax to change the line numbers for each file but you'd need to know which files have matched the pattern, and this seems too complicated.
  4. It's applied to all matching files. That's somehow logical and a bit absurd at once. But it would be consistent.
  5. Instead, introduce a syntax like pattern@tag -- which would be easier to apply to a set of files (eg. run all tests where the file path matches pattern and the test has the tag tag)

I think 1b) makes the most sense, maybe with an error message like "Line numbers can only be changed for a single file"? But 5) is interesting too.

Maybe there's a better way but I'm not seeing it.

For syntax, originally I'd thought small L for line number like l 49 but maybe :49 would match better how mix test works.

@randycoulman
Copy link
Owner

@byronalley Thanks for taking the time to outline your thinking.

TL;DR:

  • I'm open to a PR that implements 1b, where instead of an error, it's a warning (in yellow) that says something like multiple matching files; line number was ignored. The warning should probably come after the tests run, probably alongside the "Ran all test files matching ..." message.
  • I'm fine with either l or : as the command. I really like :49, but that would make command parsing tricker, so I think it would have to be : 49 instead, so maybe l is a better choice.
  • Do we need a way to clear a line number while keeping patterns?
  • I'd like to allow pattern:line as an option. If it only matches one file, then the line number is applied; otherwise, we run all matching test files and show the warning above.
  • We need to continue supporting a single relative_path:line_number or multiple relative_path:line_number options as we do today.

Details:

For option 5, see #31. Depending on how it's done, I think it would be possible to make the p and tag-related commands independent so that you could define both a pattern and a tag filter that would both be applied. We'd need some way of clearing either the pattern or the tag filter, but that doesn't seem insurmountable. The only question is how complex of a mental model it would require for the user. But that doesn't solve the line number use case.

In experimenting, I realized that there is at least one case where we emit an error message: when there are no matching patterns. So there is precedent for this. I also discovered that we already support the multiple file:line case that Elixir 1.16 supports because we pass file:line patterns straight through to mix test.

In order to support line number modification, we'll have to do more processing of the file:line pattern. so that we can manipulate it. In doing so, we'll want to maintain the current support for one or more relative_line:path patterns. We'll want to a test for the multiple case, because there likely isn't one (since ExUnit didn't support that when I wrote that code).

Depending on how we do this, it might be easy to also support a mix of pattern:line and pattern combinations in a single p command, which is not currently possible.

If we do this, we could then be a bit more clever with the line number command: look at the existing patterns, and if there is exactly one that contains a line number, then modify that line number and leave the remaining patterns alone. If none contain a line number, then when applying the pattern filter to the list of test files, check if there is only one matching file. If there is, apply the line number to it. Otherwise, ignore the line number and print the warning.

I'm still wondering if this is making things too complex for the user, but I would like to support something like this, so I'm open to some experiments/ideas/PRs in this area.

@byronalley
Copy link
Contributor

@randycoulman I like that thinking. Like you say, probably needs a bit of experimentation to get it right, but I think there's a good chance of making it work.

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

No branches or pull requests

3 participants