Add setup_scripts to cram tests #12650
Conversation
rgrinberg
force-pushed
the
cram-setup-script
branch
from
498a862 to
f3fdf4f
Compare
rgrinberg
force-pushed
the
cram-setup-script
branch
3 times, most recently
from
4891c5b to
8c98da5
Compare
Signed-off-by: Rudi Grinberg <[email protected]>
Signed-off-by: Rudi Grinberg <[email protected]> refactor: my personal tweaks Signed-off-by: Rudi Grinberg <[email protected]>
rgrinberg
force-pushed
the
cram-setup-script
branch
from
8c98da5 to
9c26f9f
Compare
|
ping @Alizter |
I'll deduplicate them.
We could add some code to detect such scripts timing out to give a proper error message. Don't think this is essential.
I'll get rid of it. I used it in an earlier version where I didn't allow for external paths. |
Alizter
left a comment
Alizter
left a comment
There was a problem hiding this comment.
Can we also have a test that shows changing the helper script after running a test triggers a rebuild. (i.e. it is registered as a dep). I want to be certain the helper script appears in the relevant digests.
|
|
||
| $ cat > dune-project << EOF | ||
| > (lang dune 3.21) | ||
| > (cram enable) |
There was a problem hiding this comment.
| > (cram enable) |
|
|
||
| $ cat > dune-project << EOF | ||
| > (lang dune 3.21) | ||
| > (cram enable) |
There was a problem hiding this comment.
| > (cram enable) |
|
|
||
| $ cat > dune-project << EOF | ||
| > (lang dune 3.21) | ||
| > (cram enable) |
There was a problem hiding this comment.
| > (cram enable) |
|
|
||
| $ cat > dune-project << EOF | ||
| > (lang dune 3.21) | ||
| > (cram enable) |
There was a problem hiding this comment.
| > (cram enable) |
|
|
||
| $ cat > dune-project << EOF | ||
| > (lang dune 3.21) | ||
| > (cram enable) |
There was a problem hiding this comment.
| > (cram enable) |
|
|
||
| $ cat > dune-project << EOF | ||
| > (lang dune 3.21) | ||
| > (cram enable) |
There was a problem hiding this comment.
| > (cram enable) |
|
|
||
| Run the test: | ||
|
|
||
| $ dune runtest |
There was a problem hiding this comment.
Can we make this test fail so that we can see it has run.
| > show_final exists | ||
| > EOF | ||
|
|
||
| Run tests to see the actual behavior: |
There was a problem hiding this comment.
We are not seeing the actual order here? Perhaps you meant to promote and cat afterwards?
|
Also can you move the tests to |
Given that the cram tests are sandboxed, the fact that setup script is copied to the sandbox should already be evidence we are depending on it. |
|
Another potential issue I see, and this one is a bit pathological, is having two scripts Obviously users doing this are being silly, but I'm interested in what error message we get. It's probably not worth being resilient against since there are plenty of other ways to break dune. The point I'm trying to make here is what happens when the script cleanup goes wrong. |
Alizter
left a comment
Alizter
left a comment
There was a problem hiding this comment.
I've pointed some things out but they are not critical. The rest looks fine to me. We will likely find any teething pains when we start using it ourselves.
2 participants
As described here #12570