Skip to content
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

Remove the build dependency from some tests. #909

Merged
merged 1 commit into from
May 25, 2022

Conversation

utam0k
Copy link
Member

@utam0k utam0k commented May 9, 2022

Remove the build dependency from some tests because it was inconvenient that we could always test only the latest builds. Instead, check in advance if the runtime is presents

@YJDoc2
Copy link
Collaborator

YJDoc2 commented May 24, 2022

Hey, this looks good , but I'm still a bit confused for why we are extracting the build step from Makefile into the CI? I'm not saying it is a bad choice to do so, but just wondering why we are doing it.

@utam0k
Copy link
Member Author

utam0k commented May 24, 2022

@YJDoc2 Nice question! The build dependencies during integration-test would have forced either release or debug builds in advance. This was too restrictive, so we removed this dependency. This made a build step necessary for CI.

In the future, I would like to refactor this CI, so that I can do the build in one place, upload the results, and reuse them elsewhere. I would also like to modify it so that unit tests are done only in one place.

@YJDoc2
Copy link
Collaborator

YJDoc2 commented May 24, 2022

👍

Nice question! The build dependencies during integration-test would have forced either release or debug builds in advance. This was too restrictive, so we removed this dependency. This made a build step necessary for CI.

Yes, currently the CI seems too monolithic, with a lot of steps repeated over and over 😓 I will try to find a way to simplify. With the amount of checks we run, (tests x 2 rust versions, integration tests, lints) it might be useful to take a look the test pipeline which Rust language repo uses, although that will need paid resources.

In the future, I would like to refactor this CI, so that I can do the build in one place, upload the results, and reuse them elsewhere. I would also like to modify it so that unit tests are done only in one place.

Copy link
Collaborator

@YJDoc2 YJDoc2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@utam0k utam0k merged commit cdcef4a into youki-dev:main May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants