Reduce the volume of expected reports files and simplify their maintenance

[fantazio]

Context

Looking at #27, there are more than 1k files changed. Only 10 of them are not tests-related. 11 are updates in Makefiles and the test engine. ~50 are new examples and ~150 are their corresponding expected reports files. The rest is only related to the reorganization of tests (moving files and updating the paths in the expected reports).

This is a perfect example of the main issue with the current test organization as a whole : the volume of expected reports files is disproportionate and its associated maintenance too.
Adding a new example file leads to adding 3 expected reports files on average in this PR. This number is actually higher in practice with almost 5 expected reports files per example file on average.

• Having one expected reports file per example file per (sub)section the example may have reports in (empty files have no effect), each with their own extension is quickly confusing and tedious.
• It is quickly discouraging to add new tests. Same for adding new scenarios.
• A lot of the expected reports are copied from one scenario to another.
• It leads to bigger and noisier PRs.
• This is cumbersome and error-prone both for the developer and the reviewer.
  There is actually a mistake in Stop relying on the cmti paths to find the corresponding source files #27: multiple "threshold" files (e.g. check/classic/examples/using_dune/unwrapped_lib/opt_args/mixed_args.mlopta1, check/classic/examples/using_dune/unwrapped_lib/opt_args/opt_args_in_higher_order_fun.mli2, ...) were added in the classic and internal expected reports.¹

Expected behavior

1. Adding new examples and their expected reports should be straightforward and PR-friendly:
   a. Limit the number of files changed.
   b. Limit redundancies in the changes.

2. Fixes and regressions should be observable. It currently is by running make check (or the individual scenarios' rules) which outputs some statistics (e.g. the number of sucessful reports and number of failed reports).

3. Adding failing examples should be possible and even encouraged to keep track of the limitations of the tool. It is currently possible but their observability requires running the tool (e.g. via make check), and they would hinder the CI and the readability of fixes and regressions.

Suggested solution

• Group all the expected reports files into 1 file per scenario.
• Store the current test results.
• The CI checks that the stored tests results and the results from a clean run match.
• The CI does not worry about the success rate.

Benefits

• No more mli, mlopta, mlio, ... extensions. The information is easy to locate, access and verify.
• This would reduce the number of expected reports files from O(n * m * k) to O(m) with n the number of example files, m the number of scenarios, and k the number of report sections in a scenario. (✅ satisfy 1.a. of the Expected Behavior)
• This would simplify the test engine a little. Its output would remain the same:

.> UNUSED EXPORTED VALUES:
=========================
./examples/using_make/advanced/inc_val.mli:1: x
./examples/using_make/advanced/inc_val.mli:2: y

./examples/using_make/bar.ml:1: x
./examples/using_make/bar_fn.ml:3: h \x1b[31m\x1b[41mNot detected\x1b[0m
./examples/using_make/baz.mli:1: z
./examples/using_make/baz.mli:3: u \x1b[31m\x1b[41mShould not be detected\x1b[0m
./examples/using_make/cond_sel.ml:13: h
...

• Storing this output along with the expected reports improves the readability of the current state of things:
  • The Not detected and Should not be detected are respectively indicator of false negative (FN) and false positive (FP).
  • Comparing the evolution of those results translates well into fixes and regressions when examples remain untouched:
    • If a FN/FP indicator disappears then it is a fix.
    • If a FN indicator appears then it is a regression.
    • If a newline with a FP indicator appears then it is a regression.
• CI would complain on a commit or PR that forgot to update the tests results:
  • Consequently, PRs will be explicit about their effects. (✅ satisfy 2. of the Expected Behavior)
  • It is ok to not have a 100% success rate. (✅ satisfy 3. of the Expected Behavior)
• Updating the expected tests results can be done very easily (simply rewrite its content with the new results).

Risks

• When adding or modifying an example, each scenario's expected reports will potentially need to be updated. (❌ does not statisfy 1.b. of the Expected Behavior)
  • The modifications should be straightforward and will be contained within only 2 files per scenario at most.
  • It is less redundant than having both the expected report files locations and content directly affected by the example file's location.
  • This remains easier to maintain IMO than introducing a dependency system between scenarios to share common expected reports and dispatching one scenario's expected reports in multiple locations again.
• Introducing FNs imply adding the corresponding example's expected reports in the right place in the expected reports files:
  • Keeping the comparison between expected and actual reports can be done out of order. This is the current situation. However, this could lead to expected reports looking too different from actual reports which would be confusing (section order may differ and reports within sections may differ).
  • If the comparison is order-sensitive (my preference):
    • One can still add a true positive (TP) to spot where the FNs should appear and discard the TP once done.
    • If the expected reports are out of order, fixing them will show that the FNs are still there and new TPs appeared instead of TPs replacing the FNs. At this point it is easy to correct the order in the expected reports.
• There are 2 files per scenario to maintain when one could be sufficient:
  • The expected reports file is the most important one and requires manual intervention to be updated.
  • The test results file is checked in the CI and can be updated automatically.
    It is directly affected by the first one so any change in the expected reports should result in a change in the test results.
    Storing the test results actually provides a better observability of the analyzer and the effects of changes.

Footnotes

1. They are ignored because their corresponding sections do not appear in the analyzer's results. Although this is actually an issue on the test engine's side rather than the organization of the tests themselves, the example is still relevant to show how easy it is to include and miss an invalid expected report file in a large volume of files. ↩