-
Notifications
You must be signed in to change notification settings - Fork 94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upload cylc-run artifact on test fail. #3939
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! 🤓 wasn't aware of the upload-artifact action, #TIL
Well this test failed here 🎉 But no artifact uploaded 😢
|
uses: actions/upload-artifact@v2 | ||
with: | ||
name: cylc-run | ||
path: cylc-run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hjoliver I think path
is relative to where the action is running. In the log lines next to the error uploading file in the last run, we have
Test Summary Report
-------------------
tests/functional/broadcast/00-simple.t (Wstat: 0 Tests: 5 Failed: 4)
Failed tests: 2-5
I think tests/functional/broadcast/00-simple.t
is relative to the working directory of the action. So the cylc-run
would be probably somewhere like /root/cylc-run/
(from the error log line suite logs can be found under: /root/cylc-run/cylctb-20201112T101740Z/functional/broadcast/00-simple/log/suite/
).
WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, this functionality hadn't been tested, yet.
Yes, my bad, looks like it's a relative path so ~/cylc-run
, /root/cylc-run
or possibly ../cylc-run
.
|
Maybe for now we just |
2f21a6b
to
6ce8691
Compare
😥 this issue is giving some trouble to fix. I bet it will be something really simple, a one-liner fix somewhere in our code base. A typical Friday 🐛 haha |
6ce8691
to
e0d9b00
Compare
Yeah I don't think my |
I think so. Maybe
? (ah, just saw your last commit defining the working directory 👍 ) |
Ah, I screwed up the
|
e0d9b00
to
0e7ead3
Compare
Still no joy. Ah, it's probably because the tests run inside a Docker container, but the Upload step doesn't. |
0e7ead3
to
b194a7d
Compare
I think you are right. But in that case you can just drop the The container has a volume mounted on I think we just need to adjust the permissions, and try to upload it (files created by Docker are created as root). What about this one @hjoliver ?
I think that would tar the |
My latest failure attempts to use
But the upload step still doesn't find cylc-run. I thought that would work 😖 I'll try what you suggested now... |
But there's no evidence in the log that the |
50c44b0
to
8a1e177
Compare
Yup, no way of copying a file out of the container in Docker (that I am aware of). That's normally done via mounted volumes. That was one of the things that took me a while to get used with Singularity. |
No, there is! I tried it locally and it worked.
|
8a1e177
to
e8a1f98
Compare
🎉 😂
|
If really stuck you could try sharing the cylc-run directory with the container to avoid having to copy stuff back. Line 248 in bc8c48c
|
Not stuck, I just got it working (although your way might be a bit more efficient). |
e8a1f98
to
07851ec
Compare
And #TIL about the |
@kinow - tests all passed, and I've verified that artifact upload works now. One review should do on this; if you agree, maybe you could merge this we can start using it to diagnose test failures better. |
(Follow up improvement might be to use a mapped volume instead of |
@oliver-sanders this just adds an artifact upload section to the bash test config. Hoping it'll allow us to diagnose the
broadcast/00-simple
test problem. See comments from here: #3933 (comment) The change looks simple but all I did was copy your section from the other file, so you'd best check it.[UPDATE: now for bash and main functional test actions]