Conversation
|
One potential issue I have identified is that a modified |
| def project do | ||
| [app: :test_stale, | ||
| version: "0.0.1", | ||
| test_pattern: "*_test_stale.exs"] |
There was a problem hiding this comment.
This is required here due to how the *_test targets are constructed in the Makefile.
|
I had some trouble with the tests. The only thing that ended up working was calling out to an external mix process and adding sleeps of 1 second in order to get around what seemed to be a combination of the low resolution of mtimes and the fact that other mix tasks repair mtimes in the future before the |
|
Managed to remove the sleeps by setting all files in the directory into the past and then touching the desired file. Improved the speed by ~40%, but still somewhat slow due to the external mix calls, which seem to be necessary due to how ExUnit works? |
lib/mix/lib/mix/tasks/test/stale.ex
Outdated
|
|
||
| defp test_helper_stale?(test_paths) do | ||
| test_paths | ||
| |> Stream.map(&Path.join(&1, "test_helper.exs")) |
There was a problem hiding this comment.
Stream here is likely just making things slower. :)
|
@antipax yes, tests are complicated because we already inside another ExUnit suite when running the tests themselves. We can review them later how to make them faster. |
|
@antipax great job as always! I have two final suggestions:
|
| assert mix(~w[test --stale] ++ opts) =~ expected | ||
| end | ||
|
|
||
| defp mix(args, envs \\ []) when is_list(args) do |
There was a problem hiding this comment.
Maybe we should put this and the functions below in the test helper since now there are two files that need them. :)
|
Ok, lookin pretty good to me now. Any other feedback? |
|
❤️ 💚 💙 💛 💜 |
4 participants
Implements #4688.
I managed to make only minimal changes to the existing test task so that it would be easy to verify that nothing was broken by the addition of
--stale, which also had the nice benefit of allowing me to put almost all the code for this feature in a separate module/file.I still have tests to write, but my manual testing is showing that this is working as I am expecting it to, so I invite everyone to review the implementation and try it out on their projects.
/cc @josevalim @fishcakez @lpil