ci: verify main itself, and check the built gem before it ships - #65
Open
tas50 wants to merge 2 commits into
Open
ci: verify main itself, and check the built gem before it ships#65tas50 wants to merge 2 commits into
tas50 wants to merge 2 commits into
Conversation
The workflow only ran on pull_request, so main itself was never verified. Two pull requests that are each green can still break main together, and release-please cuts a release straight off whatever is on main, so a break there goes out rather than being caught. Signed-off-by: Tim Smith <tim@mondoo.com>
Nothing inspects the package between a merge and RubyGems: release-please merges, the publish workflow builds and pushes, and that is the whole path. A gemspec whose spec.files glob stops matching would therefore ship silently -- the file list here is built by grepping git ls-files, so a rename of LICENSE is all it would take. Build the gem on every pull request and assert LICENSE and lib/ are in it. Verified by pointing the glob at LICENSE.txt, which the check rejects. Signed-off-by: Tim Smith <tim@mondoo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two gaps in what CI actually covers. Neither is about testing against a real
CloudStack — see the note at the bottom on why I did not go that way.
main is never verified
.github/workflows/lint.ymlonly triggers onpull_request. Two pull requeststhat are each green against their own merge base can still break
maintogether, and release-please cuts a release straight off whatever is sitting on
main— so a break there ships rather than being caught. Addedpush: branches: [main].Nothing looks at the built gem before it goes to RubyGems
The release path is fully automated: release-please merges,
publish.ymlbuildsthe gem and pushes it to GitHub Packages and RubyGems. No human sees the package
in between.
spec.filesis built by greppinggit ls-files:That is fine today — I built the gem and it contains
LICENSEand all sixfiles under
lib/. But it is a regex against filenames, so renaming the licensefile to
LICENSE.txtorLICENSE.mdwould silently ship every subsequentrelease with no license in it, and the first anyone would know is a downstream
packaging complaint.
Added a job that builds the gem on every pull request and asserts
LICENSEandlib/kitchen/driver/cloudstack.rbare in the packaged file list. I checked itactually catches the thing by pointing the glob at
LICENSE.txt:What I looked at and left alone
Unit tests across the supported Rubies: already fine. The shared
test-kitchen/.githublint-unit.ymlrunsbundle exec rake teston Ruby 3.1,3.2, 3.3, 3.4 and 4.0, which matches the gemspec's
required_ruby_version >= 3.1exactly. It also runscookstyle --chefstyle, yamllint and markdownlint.No orphan tests.
rake testusesspec/**/*_spec.rb, so every spec file inthe repo runs,
spec/integration/lifecycle_spec.rbincluded. There are no testscripts sitting outside that pattern.
Integration against a fake CloudStack: already here, and #62 owns it.
spec/integration/lifecycle_spec.rbalready stubs the HTTP boundary and drivesa real
Kitchen::Instancethrough create/status/destroy, so Test Kitchen'slifecycle, the driver, the client gem and real response parsing all execute
against a faked cloud on every pull request. Broadening it — signature
verification,
doctor, a repeat destroy, theassociate_public_ippath — wouldmean rewriting exactly the file and exactly the client layer that #62 is
replacing. That is worth doing, but on top of #62, not underneath it.