-
Notifications
You must be signed in to change notification settings - Fork 354
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
clean up the shell script for youki integration test. #600
clean up the shell script for youki integration test. #600
Conversation
Codecov Report
@@ Coverage Diff @@
## main #600 +/- ##
=======================================
Coverage 70.18% 70.18%
=======================================
Files 82 82
Lines 10909 10909
=======================================
Hits 7656 7656
Misses 3253 3253 |
8e0fbd3
to
664651d
Compare
664651d
to
b02f1b8
Compare
Hey, I know you are still working on this but some points :
|
Also as this is the aftermath of #582 , let me know if there are any big issues with it, as in your comment #582 (review) you have said If there are any issues that need to be fixed, let me know I'll try my best 😅 |
@YJDoc2 |
@YJDoc2
|
Anyway, in this PR, I wanted to make sure that shell scripts can be run in any directory in order to organize the directory structure in the future. |
That PR was mainly for adding the test and runtimetest, so it was finished. However the script could have been improved (as seen in this PR 😅 ). The implementation of build command was nothing different, the way it worked was it would build and copy over all the binaries, and if the $2 arg was "build" it would stop there and exit the script. Otherwise, it would continue to execute the tests. In that sense, the build command was more of an early exit, rather than a command.
I agree, I felt this strongly as I was working on the runtimetest tool, and especially when I had to separate it into its own crate. Even now it has an issue where the I feel (in the long term) a good idea would be to separate test framework, integration tests and runtimetest into a separate repo, like opencontainers/runtime-tools. That way we can include that as a submodule like we have included the runtime-tools. Either way we do need to have a better structure, one suggestion would be to move the whole youki workspace, including its cargo toml and target into a directory of its own so the structure would be something like /youki_workspace/crates/... that way we might be able to separate integration test into its own workspace , but I'm sure you would have a much better idea than me.
Not at all ; however I'd prefer a single person to work on this, rather than multiple people creating conflicts for each other. So if you want to work on this, please go ahead, I'll work on some other tests or something else. If you want me to work on this, I don't mind it either.
This is great!
|
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.
LGTM 👍
No description provided.