ocaml icon indicating copy to clipboard operation
ocaml copied to clipboard

add a `cold` action to disable tests by default

Open gasche opened this issue 2 years ago • 4 comments

We are concerned with the overall running time of our testsuite that keeps growing. On the other hand, the testsuite is the best place to accumulate factual knowledge about the rest of the compiler codebase, because we keep checking that it works.

In the past we have discussed adding testcases for known-bugs, to be able to (1) preserve their knowledge in a maintained place and (2) notice if they suddenly get fixed by compiler changes. This is in tension with the requirement to keep the testsuite fast. See #11439 for an example of this tension.

I propose to add a mechanism to disable some "cold" tests by default, so that they are not run by automated CI processes (unless explicitly configured to do so). ocamltest already uses actions as a conditional-test-selection mechanism, for example the frame_pointers action will skip tests unless frame pointers are available. I propose to add a cold action that skips unless the environment variable OCAMLTEST_COLD_TESTS=<bool> explicitly requires them.

In the future we could think of CI processes configured to run those tests on a weekly basis, but for now it would be the responsiblity of our glorious release manager to manually run those tests from time to time.

Notes:

  • What if we forget to run them entirely? Well, that would be no worse than the current status where we don't have those tests in the first place.
  • Reusing ocamltest actions in this way does not give an easy way to select only the cold tests to run (which would make sense if we are configuring a test process that runs in addition to the usual testing routine). In the future we could add a filtering option to the ocamltest driver to only run tests that use a certain action or set a variable to a certain value. This is not required for a first iteration that is already useful.

gasche avatar Jul 24 '23 07:07 gasche

@gasche and I discussed this a bit, offline.

I'd prefer a more general, tag-based approach which I know @gasche did consider, too.

We discussed what shold be tagged: files or individual tests? As long as we considered the right approach was to tag whole files, we entered discussion on syntax which were not that nice. Among other things, we considered adding the file tags right at the beginning of the test block but that felt odd. Then I suggested that we could introduce a tags variable that would have to be defined at top level to be taken into account, whcih @gasche immediately found not principled enough and, in a way, contradicting my own quest for principles.

Finally, @damiendoligez remarked that it would actually make sense to tag specific tests rather than files.

I am thus proposing again the introduction of the tags variable because it would have a semantics wherever it is defined.

One would then add command-line options to ocamltest to enable / disable tags, and some machinery in testsuite/Makefile to add such flags to an environemnt variable that would then be passed on to ocmaltest through its command-line.

How does that feel?

shindere avatar Jul 24 '23 14:07 shindere

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

github-actions[bot] avatar Jul 26 '24 04:07 github-actions[bot]

What about a lowtech approach of having a cold subdirectory?

Octachron avatar Jul 26 '24 07:07 Octachron

Having a cold action that skips unless some environment variable is set is pretty low-tech, I can implement it in half an hour, and rather more convenient than having to shove completely unrelated tests in the same place. The reason why I have not implemented it is that @shindere is against the idea.

gasche avatar Jul 26 '24 08:07 gasche

This issue has been open one year with no activity and without being identified as a bug. Consequently, it is being marked with the "stale" label to see if anyone has comments that provide new information on the issue. Is the issue still reproducible? Did it appear in other contexts? How critical is it? Did you miss this feature in a new setting? etc. Note that the issue will not be closed in the absence of new activity: the "stale" label is only used for triaging reason.

github-actions[bot] avatar Aug 04 '25 04:08 github-actions[bot]