Please stop running `dune runtest` in the CI

[toots]

Hi!

I would like to kindly request the opam reporitory maintainers to stop running dune runtest in the CI.

The way I see it, this is creating an anti-pattern:

• Tests are intended for the project's internal consumption to make sure that the maintainer can track progress, regression etc.
• In many cases, running the tests intended to cover a large project will require specific software and hardware setup that only the maintainers should be in charge of understanding/clarifying.
• The opam packaging CI should only be concerned about the shape of the built package, does it install the right files, builds etc and how a new package impacts the rest of the packaging ecosystem.
• Running dune runtest in the opam CI creates maintenance burden on opam repository maintainers who then have to figure out if a test failure is legitimate or not even though they have no prior knowledge of the underlying package's logic.
• ⚠️ On the package maintainer side, and this is perhaps the most important, this creates a big anti-pattern: there is now an vested interest in moving the packaging's test outside of the dune runtest framework to prevent their exposure during the opam package review process. However, the dune testing framework is not just a convention for running tests, it's also a convention for installing testing dependencies so, as much as possible, dune runtest and dune testing dependencies should stay hands-in-hands.

Thanks y'all for y'all hard work and consideration and happy new year! 🎉

[shonfeder]

In general

I am not in favor of this change, because running these tests in CI amounts to ecosystem-level integration tests allowing us to ensure updates to the ecosystem do not introduce regressions or unintended behavioral breakage. These checks allows us to catch regressions all the time. If we remove this check from the opam repo CI without an alternative, we are likely to see a swift degradation in the integration of the ecosystem.

It is also worth noting that in the CI we do not run dune runtest we run opam install --with-test. But, when generating opam files, dune makes the decision to assign dune runtest -p as the with-test build target. So, if this request is properly targeted at the opam repo, it is actually the request "don't install packages --with-test during revdep testing". Otherwise, it may be better targeted at dune.

Addressing particular points

The opam packaging CI should only be concerned about the shape of the built package, does it install the right files, builds etc and how a new package impacts the rest of the packaging ecosystem.

How can we check "how a new package impacts the rest of the packaging ecosystem" without running the tests of a package's reverse dependencies to see if regressions were introduced?

Tests are intended for the project's internal consumption to make sure that the maintainer can track progress, regression etc.

The presence of opam's --with-test flag, which allows clients of packages to run the tests during installation, seems to be evidence counter to this claim. This may be how you view tests in your projects, but I don't think it is how opam (the executable or repo) or dune has viewed the runtest targets historically.

In many cases, running the tests intended to cover a large project will require specific software and hardware setup that only the maintainers should be in charge of understanding/clarifying.

I think this shows that we do need to normalize thinking about different kinds of test targets. I agree that these kinds of test cases are not a good fit for the opam repo CI tests, or for the opam install --with-test target.

Running dune runtest in the opam CI creates maintenance burden on opam repository maintainers who then have to figure out if a test failure is legitimate or not even though they have no prior knowledge of the underlying package's logic.

The way to alleviate this burden, IMO, is to have reliable and robust tests associated with the runtest alias.

On the package maintainer side, and this is perhaps the most important, this creates a big anti-pattern: there is now an vested interest in moving the packaging's test outside of the dune runtest framework to prevent their exposure during the opam package review process

We cannot prevent people from bypassing the checks for integration in the way they define their projects. But, IMO, this is a posi-pattern if people are moving out the sort of tests you describe, since those indeed aren't unit tests, and opam install --with-test is meant to "Build and run the package unit-tests" (according to opam install --help).

there is now an vested interest in moving the packaging's test outside of the dune runtest framework to prevent their exposure during the opam package review process

For the appropriate sort of tests, package authors who do so are depriving their packages, and the ecosystem, of the benefits of the regression tests we run on their dependencies. So I would think the vested interest goes the other way: they should want reliable, meaningful tests associated with this target in their projects, since it protects them from breakage due to changes in their dependencies.

Counter proposal

I think there are ways we can address the pain points you mention without giving up the benefits of these integration test.

Here are three key ideas in this direction:

1. Eventually we should move the opam repo ci tests "to the left", earlier into the SLDC. We should still have the checks here as a safeguard, but if we make it easy for devs to run the devdep checks in their development flows, we can reduce a lot of the noise here. (This has been planned for a long time, and we have some WIP in this direction, but resources and time are scarce.)

2. We need dedicated time to clean up defective packages that always fail revdeps for irrelevant reasons.

3. We should introduce a convention of having two different kinds of test targets for packages: (a) developer-only k"integrations tests" (of the sort you mention as problematic for runtest) and (b) unit tests, which is what opam install --with-test runs.

   I think it would be most practical and least disruptive to introduce a new standard alias in dune for tests like (a), And runtest should remain focused on the unit level. But this is an issue for dune, as I don't think opam or the opam repo have a reason to know about (a).

---

I look forward to hearing your (and other's) thoughts: while I don't agree with the proposed solution, I think there are certainly problems to be solved and improvements to be made here.

[avsm]

It's extremely useful to have some tests within a project to check, since the test coverage of opam-repo-ci is invariably greater than that of most individual projects (e.g. a serious s390x error found on #29148).

However, your point about having tiers of tests is well taken; this is a common problem I've observed in my own projects where I have:

1. quick unit tests
2. more elaborate network or storage level tests
3. end-to-end tests that require a specific setup
4. benchmarks
5. fuzztests

of these 1) can obviously be run in opam-repo, and 2) can sometimes.
But 3) ought to be skipped from the opam-repo and saved for another build alias.
4) requires a hardware-isolated deployment
5) runs for arbitrary periods of time and needs a new opam switch

So there's some scope in coming up with more 'standard' test aliases for dune, I think...

[raphael-proust]

However, your point about having tiers of tests is well taken; this is a common problem I've observed in my own projects where I have:

1. quick unit tests

2. more elaborate network or storage level tests

3. end-to-end tests that require a specific setup

4. benchmarks

5. fuzztests

I've seen projects also include tests amounting to "is the README up-to-date?" (README or sometimes other pieces of semi-autogenerated pieces of documentation… think mdx or such). These cause a lot of issues in the opam repo CI because seemingly innocuous changes in the doc gen tools (e.g., changing line-break in prose) cause test errors.

[dra27]

It's extremely useful to have some tests within a project to check, since the test coverage of opam-repo-ci is invariably greater than that of most individual projects (e.g. a serious s390x error found on #29148).

Additionally, there are users who (may) choose to install packages with --with-test, so opam-repo-ci is also checking the health of package metadata.

I agree this does seem to be a request for Dune, rather than opam-repository. A very easy first step (which I don't think is already present) would be the ability to specify in dune-project that @runtest should not be added to generating opam files, either globally or at the individual package level.

[raphael-proust]

I think we need a good set of examples or a tutorial on how to mark different tests as intended for different contexts. There are multiple possible approaches to this and it's not obvious how to go about it.

I think what I'd rather do in my packages is change the meaning of the alias runtest depending on the dune profile or env. Ultimately, I want to run dune build @runtest (or equivalent) during development because that's the common way it's done everywhere and I don't want to have different targets-aliases based on different projects. But the runtest alias should mean different things in dev mode and in release mode.

An alternative is to make some tests conditionals on some environment (OPAM_CI or maybe CI is set by the opam repo ci, can't remember which). The issue is that as @dra27 mentions users may install with --with-test which wouldn't be covered by this method.

Yet another alternative is that not all tests should appear in the release tarballs. You could very much have dev-tests/ and release-tests/ directory, only the latter being shipped. It requires a bit of setup for the releases, and it only works at a certain granularity, but that'd be ok for me too.

Any idea on what to recommend as a go to method? Suggestions?

[toots]

Thanks for all the constructive comments!

Without over engineering too much I'd say that having a broad category for dev tests and a restricted one for production tests would work.

Test dependencies would be expected to cover both categories.

I'm not sure if that is a dune specific issue. opam does have this support baked in so it makes sense to give it a semantics at least in the specs/docs so that packagers and tool authors can act accordingly.

[shonfeder]

I'm not sure if that is a dune specific issue. opam does have this support baked in so it makes sense to give it a semantics at least in the specs/docs so that packagers and tool authors can act accordingly.

opam's support is thru the with-test variable. Package authors have complete control over what gets run when users opam instal --with-test, as is this determined by the use of the with-test variable in the build field. E.g.,

opam-repository/packages/liquidsoap/liquidsoap.2.4.0/opam

Line 130 in 6bda456

"@runtest" {with-test}

specifies a build command that includes the runtest alias as a build target if the installation is run --with-test. Package authors are free to change this to whatever targets they think are suitable, or to use a completely separate command for installation with tests.

On the dune side, it is opinionated about what targets are used for this when it is asked to generate opam files (tho this can be overridden by opam.template files).

[toots]

I'm not sure if that is a dune specific issue. opam does have this support baked in so it makes sense to give it a semantics at least in the specs/docs so that packagers and tool authors can act accordingly.

opam's support is thru the with-test variable. Package authors have complete control over what gets run when users opam instal --with-test, as is this determined by the use of the with-test variable in the build field. E.g.,

opam-repository/packages/liquidsoap/liquidsoap.2.4.0/opam

Line 130 in 6bda456

"@runtest" {with-test}
specifies a build command that includes the runtest alias as a build target if the installation is run --with-test. Package authors are free to change this to whatever targets they think are suitable, or to use a completely separate command for installation with tests.

On the dune side, it is opinionated about what targets are used for this when it is asked to generate opam files (tho this can be overridden by opam.template files).

Thanks that makes sense.

I'll bring it up with dune to see if they/we can find a better default. I believe the default test target should serve the developer first so it should be opt-in for package-specific tests.