`x.py test` should run tests for changed code first

[thomcc]

This is mostly just to speed up iteration time in a {edit,compile,test} cycle. It also matters more for doctests than normal tests, since these are the tests that are slowest. It would also be okay if this behavior were behind a non-default flag.

I gather (from @jyn514) that this is tricky because the testrunner doesn't know the right module name in all cases, but it would probably be fine to just take a stab at it, since eventually everything should still get run anyway.

[Michael-F-Bryan]

@thomcc do you have any idea how this might be implemented?

If this is just intended as an optimisation, one idea is to look in the build directory for a well-known file and see when it was last modified, then we could traverse the directory of whatever you are building, looking for files which have been modified since. While you can't get the "true" module name without actually parsing files, the vast majority of Rust modules will follow the convention of naming the file after its module.

From there we could effectively run cargo test -- foo::bar foo::baz && cargo test to make sure the modules which have changed get tested first.

@jyn514 would something like that be feasible in rustbuild? A more complete solution would be to have the functionality built into cargo and Rust's test runner so every Rust project could benefit, but that's 1-2 orders of magnitude more effort.

[jyn514]

There are a few issues here:

• Even if we implemented the feature, it wouldn't help much because doctests don't show which test failed until all of them fail. It would be nice to do something like ninja where it shows the errors as it goes:

  > ninja
  [104/385] [BISON][OctoParser] Building parser with bison 3.5.1
  /home/joshua/src/yottadb/octo/build-ninja/parser.y: warning: 19 shift/reduce conflicts [-Wconflicts-sr]
  [385/385] Linking C executable src/test_make_empty_query_response
  >
  

  I think this would need changes in rustdoc, but not cargo.
• I would indeed prefer to upstream this into Cargo. I'm ok with adding this to rustbuild temporarily, but not if it would be almost as much work as adding to cargo, which I think it might be.
• On the implementation side, traversing the source directory seems fine, although I'm slightly worried it will increase the overhead of running tests. We could make this optional (either opt-out or opt-in) and work on the overhead later if necessary.
  Why would this need to look for a well-known file in the build directory? x.py knows what it's testing already.

[Michael-F-Bryan]

Why would this need to look for a well-known file in the build directory? x.py knows what it's testing already.

If you know that file was updated in the last build you can use its "Last Modified" property to figure out which *.rs files were modified since the tests were run last.

Even if we implemented the feature, it wouldn't help much because doctests don't show which test failed until all of them fail.

I was doing a bit of experimenting locally and it seems like failing doctests will be reported when they occur, although you don't see the output (i.e. the panic message or compile errors) until all of them have finished.

2021-01-22_08-58-08.mp4

I would indeed prefer to upstream this into Cargo.

One implementation would be to modify rustc so you can query the "Last Modified" property for the file attached to a particular span. That way both the code setting up the test runner and rustdoc would be able to sort by time since last modified when deciding the test order.

Not gonna lie, that sounds like a non-trivial amount of effort... Although it could have the potential to greatly reduce CI/edit-compile-test times across the ecosystem when there is a test failure and people want to fail fast.

[Mark-Simulacrum]

I will caution that this feels like a pretty big project, and one that will make x.py even harder to test due to blowing up the possible orderings in which things run. I think I would prefer that we not land this.