Reftests add repository layout test - #6941
Conversation
NathanReb
left a comment
NathanReb
left a comment
There was a problem hiding this comment.
The test is quite large and I wonder if a slightly different structure could improve its readability and eventually even eliminate some duplication, potentially reducing the overall size.
Currently, if we leave out section IV, the test is roughly organised by setting up a repo with every layout under test (e.g. packages/good/good.1/opam or packages/direct-root.1/opam) and then run commands that will show the behaviour on each individual layout at once.
I'd like to suggest trying the opposite, i.e. for each layout run all the commands we want, describing how it should behave for said layout and then showing how it actually behaves in the commands output.
That would allow to reduce the size of the commands output (especially with the debug logs, its hard to keep track of everything) and keep everything closer to the documentation/comments that are relevant to this particular test.
That would also allow to describe what the layout actually are which could clear out some cases that may look like unintentional typos such as packages/i/n/e/r/very-inned-n-nv/very-inner-n-nv.1 where very-inned-n-nv VS very-inner-n-nv.1 could either be a typo or part of the test.
I suspect that trying out such a structure might show that some layouts actually exhibit the same behaviour because they go through the same code path in certain situations.
What do you think?
|
I like the idea! |
|
Updated with the new structure. |
558aebf to
dba840b
Compare
dba840b to
44ff6c8
Compare
44ff6c8 to
148177f
Compare
|
Updated, and updated the main comment |
148177f to
0b57219
Compare
0b57219 to
bf1e1d4
Compare
kit-ty-kate
added
PR: QUEUED
| ### # we add change in opam.xxx | ||
| ### sh -c "echo >> REPO/packages/wrong-opamf-updated/wrong-opamf-updated.1/opam.xxx" | ||
| ### :C:1: Update of repositories | ||
| ### OPAMDEBUGSECTIONS="FILE(opam) opam-file RSTATE UPDATE" OPAMDEBUG=-3 opam update default-strict --strict | unordered |
There was a problem hiding this comment.
are the | unordered still useful after the addition of the Array.sort?
There was a problem hiding this comment.
i've removed them. Let's see if it's indeed the case everywhere
It is mainly for logging, like that we have an harmonised output.
bf1e1d4 to
b2b8534
Compare
kit-ty-kate
removed
the
PR: QUEUED
|
Ignoring ocaml-benchmarks |
3 participants
Add a test that shows the handled paths for opam packages, how and if they are read or processed by opam client (update & admin commands).
This mechanism was reworked by #6614 that introduced incremental loading (from diff instead of reading again the whole repository).
It highlights and permit to follow some errors that were unseen during #6614 and that are resolved by #6871 & #6924 (previous handled paths are seen in first commit[1]).
This PR adds the test, that is good to have before #6625 rewriting, to ensure non change on behaviour (even if erroneous).
However, in this PR we change a little bit the behaviour: first by adding a new logging to differentiate load from dir & load from diff calls ; and we sort the files read from disk during loading from dir to ensure an cross system reftest output (especially with
---strictthat error at the first error encountered).It also fixes a bug, a different behaviour between Unix & Windows. Stdlibopen_infunctions opens directories on Unix but error on Windows. Now,OpamSystem.open_in*error on all platforms if the filename is a directory.[1]
The first commit is here for my personal use. It will be removed from the final version., it higlights the output of the test before before incremental loading merge (at commit f370351). It will be removed on merge.The commit that contains the result of the reftest before incremental loading is in on top of the branch rjbou/bef-load-from-diff-reftests-repository-layoutIn the story of #6625
Queued on #6995