randycoulman / mix_test_interactive

Interactive watch mode for Elixir's mix test. https://hexdocs.pm/mix_test_interactive/
MIT License
73 stars 12 forks source link

Switch between file and line number #65

Open a8t opened 1 year ago

a8t commented 1 year ago

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 commented 1 year ago

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

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

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

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

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

TL;DR:

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

@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.