Adjust duration_by_test definition to reflect new use

[CyberiaResurrection]

As at the status quo, whichever call item ran last (usually 'teardown') had its time overwrite the test in question's entry in duration_by_test.

The simplest way I could think of to avoid that was to add each call item's duration to the total in duration_by_test. Changing duration_by_test to a defaultdict simplified the bookkeeping.

[CyberiaResurrection]

@Otto-AA , @boxed , I think this addresses #427.

[Otto-AA]

Thanks for your investigation! It looks good, but I think we should (re)set the duration to 0 at the test start instead of using a defaultdict.

I am not sure if it is an issue currently, but maybe we could run the stats collection twice, or load existing stats from the json. Without resetting the duration to 0, we would collect some test duration in the first run, and in the second run we would add to the test duration instead of starting again from 0.

I think we could use the pytest_runtest_logstart hook to set the value to 0 (I think this hook is called at the start of the test, see https://docs.pytest.org/en/7.1.x/reference/reference.html#pytest.hookspec.pytest_runtest_protocol)

[CyberiaResurrection]

@Otto-AA - are you meaning between different test runs?

If so, isn't that (at least currently) mitigated by only passing the StatsCollector plugin to the stats-collection test run?
Or are you meaning unconditionally clearing duration_by_test at the start of stats collection?

[Otto-AA]

I'm thinking about what happens when duration_by_test is already filled before run_stats is called. Previously we would have replaced existing test durations in duration_by_test (by using =), now we would add to it (by using +=)

This could happen if we call run_stats multiple times within a single command execution (we probably don't do this currently, maybe with the mutmut browse). Or by loading the stats from the json into duration_by_test and afterwards running run_stats (see

mutmut/mutmut/__main__.py

Line 799 in 5ab6b77

did_load = load_stats()

). I think the latter case could also be fine because then we do not run it on all tests.

But it's easier to verify, and less likely to break in the future, if we set mutmut.duration_by_test[item.nodeid] = 0 in a pytest_runtest_logstart hook in the run_stats method. So maybe your implementation is already working in all edge cases (I think so, but honestly don't know), but for me it seems easier to verify its correctness if we reset the duration to 0.

[CyberiaResurrection]

@Otto-AA , does that latest commit "Reset test duration at start of each test" do what you were thinking of?

[Otto-AA]

Yes, looks good. Feel free to merge it if the stats still works with the reset (I can't test it locally).

You could also make the defaultdict a dict again if you want, it should work either way.

[CyberiaResurrection]

I kept the defaultdict, following your "belt-and-braces" philosophy, @Otto-AA .