-
-
Notifications
You must be signed in to change notification settings - Fork 37
Smoke tests (first pass, take 2) #331
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
deivid-rodriguez
left a comment
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.
This is great, just had some minor comments.
I'm afraid all these scripts will tend to get out of date and break if we don't run them often in CI, so it's a pity that discourse is so slow since the rest of the tests seem like they could run on every PR?
Anyways, if we remember to run this on every release, I think that's good enough 👍.
This ensures that that Docker build context stays small.
This just builds rv, which we use in various testing and CI contexts - it's different from bin/rv which builds *and runs* rv.
This makes this build script's name and purpose clearer
Per the changes in #323
We can start with the Manual web trigger
1) rv determines the Ruby version from the Gemfile.lock 2) rv installs this version of Ruby 3) rv ci using the newly installed Ruby version
This was failing in Windows.
Per a code review comment
This was left over from the previous ruby-build approach.
|
Thanks for getting this set up and working! My main impression is that this is very complicated, but I'm having trouble figuring out why.
Zooming way back out, my understanding of the goal here is: 1) collect several example lockfiles, 2) ask rv to install those lockfiles to make sure that they work. Is that right? If those are the only goals we need to satisfy, I would suggest removing Docker entirely, and just having one script per smoke-test. Then script can be run in a separate GitHub Actions matrix job. I've rebased and added a commit that shows what I mean just for Diaspora. Hopefully that makes sense, but feel free to ask questions! |
Per the PR feedback - this removes Docker from the workflow, which I used during development to see it all working correctly locally and in CI. It's not needed in CI, and if needed locally, can be run via the "docker run" command that's there in comments.
|
Thanks @indirect! Yeah this was a bit of a meandering journey - I used Docker initially because I knew it would achieve parity between local work (Mac) and CI (Linux), but now that we can see it all working correctly, it makes sense to simplify it. These changes are pushed now. |
|
To be honest I liked the previous (admittedly more complex) setup, so that I could simulate all these tests locally from macOS.
I don't think it built
That's a really good point, I had the same thought when looking into this but finally decided that |
|
@deivid-rodriguez I've added |
(This PR replaces #316, which was based on my
rvrepo fork, from before I was granted direct access to thervrepo)Here's a first stab at an
rv clean-install"smoke tests" suite for some popular / reference Ruby projects:If this looks roughly aimed in the right direction, we can iterate and add more projects. (and if not, we can iterate and take a different approach!)
Tl;dr of how it works:
rvrvbinary into the Dockerfilervdetermines the project's Ruby version fromGemfile.lock, courtesy of our work in Use Ruby version from Gemfile.lock as a fallback #325rvinstalls the project's Ruby versionrv cito clean-install the projectHere's an overview:
bin/smoke-testsdir with scripts and Dockerfiles for the various Ruby projects - this is meant to keep the test configs separate from the rest of the codebase/testsdir instead? I am relatively new to Rust, so I'm not sure what the conventions are for stuff like this.rvinstall a given project's Gems?" - so we sort of need to hackrvinto the projects' individual installation workflows..dockerignorefile, in order to keep the Dockerfiles' context and sizes smallsmoke_tests.ymlfile, to run these in GH Actions. It will only run via the Actions' web UI's manual trigger button; it isn't needed for most pull requests.And a few minor, semi-related things that I added along the way:
Gemfile.lock, to improve our tests and parsing coverage for another popular project. This was related to Add Gemfile parsing for git refs and tags #313 which was a side quest from this PR.bin/integration-tests/alpineandbin/integration-tests/archscripts, so that we can run the Linux integration tests locally tooAnd for fun, I added an idea of a "baseline" test, to do a native-Ruby tooling install as well - this will let us start to see rv's speed benefits.