Skip to content

Manifezt: handle dependencies of tezt/tests/main.exe

Romain requested to merge nomadic-labs/tezos:romain-manifezt-3 into master

What

This MR ensures that Manifezt says "run all test" if dependencies of tezt/tests/main.exe, such as tezt/lib_tezos, are modified. Dependencies which are _tezt_lib libraries are excluded from the list of dependencies to consider.

Why

Let's say you modify tezt/lib_tezos/node.ml. All tests that use the Node module may now be broken. So they have to be run. The manifest doesn't know which tests depend on Node, so (after this MR) it overapproximates and says that all tests must be run.

Note that for test executables other than tezt/tests/main.exe, it already works before this MR. For instance, modifying a dependency of src/lib_base/test will cause those tests to be selected. But tezt/tests/main.exe has a special status in the manifest and if you modify its dependencies, tests from tezt/tests/main.exe are not selected before this MR.

We assume that _tezt_lib libraries, such as src_proto_alpha_lib_protocol_test_unit_tezt_lib (defined in src/proto_alpha/lib_protocol/test/unit/dune), only register tests and not helpers for other tests in other directories. With this assumption, we can exclude those libraries from dependencies that trigger all tests. If we did not do this, modifying any test would cause all tests to be run.

How

The generate function of manifest/manifest.ml now expects the list of dependencies that should trigger all tests to be run. This list is already defined in manifest/main.ml, so we just give it a larger scope to be able to pass it to generate.

Then we simply call our existing dependency analysis on this list of dependencies to see if they have changed. If they have, Manifezt just returns the TSL expression true to cause all tests to be selected.

An alternative implementation could have been to filter the list of dependencies of main.exe to remove those that end in _tezt_lib, but it was messier.

Future Work

Our assumption on _tezt_lib libraries means that when one defines let x = tezt ... in manifest/manifest.ml, x is not used as a dependency somewhere else. Currently, this is true except for src/lib_store/unix/test, which is used by src/lib_store/unix/test/bench and src/lib_store/unix/test/slow. The helper parts of src/lib_store/unix/test should thus be extracted as a separate library. Then tezt can be changed to return unit to enforce the invariant.

Note that:

manifest/manifest --manifezt src/lib_store/unix/test/assert_lib.ml

does list all files in src/lib_store/unix. Remember that this MR fixes the special case of tezt/tests/main.exe. So src/lib_store/unix/test/main.exe was already correctly handled. (test/bench and test/slow are not run in the CI so they do not cause a problem either.)

So this future work is not urgent but we'll probably want to do it in the future for safety.

Manually testing the MR

Start by compiling manifest:

make -C manifest

Then run some queries like:

# should return "true" (run all tests) (this is new with this MR)
manifest/manifest --manifezt tezt/lib_tezos/node.mli
# should return "node" (run all tests with tag node)
manifest/manifest --manifezt src/bin_node/main.ml
# should return a rather complex expression
manifest/manifest --manifezt src/lib_store/store.mli
# should return "false" (run no test)
manifest/manifest --manifezt

Checklist

  • Document the interface of any function added or modified (see the coding guidelines)
  • Document any change to the user interface, including configuration parameters (see node configuration)
  • Provide automatic testing (see the testing guide).
  • For new features and bug fixes, add an item in the appropriate changelog (docs/protocols/alpha.rst for the protocol and the environment, CHANGES.rst at the root of the repository for everything else).
  • Select suitable reviewers using the Reviewers field below.
  • Select as Assignee the next person who should take action on that MR
Edited by Romain

Merge request reports