Migrate the all Makefile-based tests to the newer dune-controlled cram-like tests / Windows fixes #4504
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AltGr
force-pushed
the
reftests-all2
branch
2 times, most recently
from
0088845
to
20d08b4
Compare
Merged
AltGr
force-pushed
the
reftests-all2
branch
3 times, most recently
from
b9f6605
to
567cb24
Compare
AltGr
force-pushed
the
reftests-all2
branch
4 times, most recently
from
710bf02
to
bda80d1
Compare
dra27
reviewed
Mar 5, 2021
src/client/opamAuxCommands.ml
Outdated
| @@ -490,12 +490,11 @@ let check_and_revert_sandboxing root config = | |||
| match OpamFilter.commands env sdbx_wrappers with | |||
| | [] -> config | |||
| | cmd::_ -> | |||
| OpamFilename.with_tmp_dir @@ fun tmp -> | |||
| let test_file = OpamFilename.(to_string Op.(tmp // "check")) in | |||
| let test_file = "$TMPDIR/opam-sandbox-check-out" in | |||
There was a problem hiding this comment.
This feels dangerous - what happens if TMPDIR is unset?
There was a problem hiding this comment.
The sandbox script is expected to set it; but indeed that should be documented...
There was a problem hiding this comment.
This is in order to get canonical output, right? Rather than altering the command, wouldn't it be better to alter OpamProcess.verbose_print_cmd to detect the string and do the conversion instead?
There was a problem hiding this comment.
We did something similar with negative debug levels stripping out the timestamps...
src_ext/Makefile.sources
Outdated
| @@ -49,8 +49,8 @@ MD5_result = 1b82dec78849680b49ae9a8a365b831b | |||
| $(call PKG_SAME,result) | |||
|
|
|||
| # NB If minimum OCaml version for Dune changes, update DUNE_SECONDARY in configure.ac | |||
| URL_dune-local = https://github.com/ocaml/dune/releases/download/2.6.2/dune-2.6.2.tbz | |||
| MD5_dune-local = 8fe40bef051975e798537d3450b61129 | |||
| URL_dune-local = https://github.com/ocaml/dune/releases/download/2.8.0/dune-2.8.0.tbz | |||
|
SPECTRUM IS GREEN! Congratulations, @AltGr! |
dra27
reviewed
Mar 5, 2021
This was referenced Mar 8, 2021
dra27
reviewed
Mar 8, 2021
| Unix.close_process_full process |> ignore; | ||
| fatal e; |
There was a problem hiding this comment.
This should be first, I think - I think Unix.close_process_full can also raise?
The existing `reftests.ml` are responsible for discovering the tests and running them, and a Makefile action performs the diff. This splits the work a bit more so that it plays better with dune (and becomes compatible with `dune promote`): - a `gen` executable reads the tests directory and outputs rules to execute tests. In addition, it determines the versions of opam-repository that are necessary, and outputs rules to download and unpack them (these will be shared across runs, and between tests that are based on the same version). - a `run` executable executes a single test with enough context passed as arguments: the path to the development `opam` binary, the test file, and the opam repository contents. - the generated rules are checked in the repository as `dune.inc` because dune needs to know them statically, but they are checked: if a test file is added, `dune runtest` will compute the new rules and they can be promoted into `dune.inc`. Reftests are set up using aliases so that they can be run individually: - each test has its own alias (e.g. `dune build @reftest-conflict-camlp4`) - all tests are attached to @reftest (`dune build @reftest` runs all) - this alias is attached to @runtest too (so `dune runtest` will run these)
It was the largest time cost, now the reftests can be run in a very reasonable time.
It's technically a mask, although in this case only 0 (files already in sync) and 1 (files all sync'd with no issues/warnings) should be accepted.
Should help a lot on Windows. The test themselves will need an update since rewrites now need a custom syntax rather than grep/shell piping
no more shell pipe sed grep awk hacks \o/
- fix some cases with error return values - fix unordered output with multiple identical lines
Some tests have unordered output ; put them back to a real order though, some parts were mixed up because of the older shell awk/sort hacks. It makes more sense to have retrievals before installs...
in particular, call scripts through their interpreter explicitely.
otherwise network accesses won't work
|
cleaned up! Should be ready to merge |
|
hoorray! \o/ |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Wip additional work on #4405