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

Use GitHub Actions for CI #96

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Use GitHub Actions for CI #96

wants to merge 6 commits into from

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Nov 11, 2024

This allows us to remove a workflow in Jenkins.

This has been unused since c7cad93.

Fixes: c7cad93 ("Fixes #32709 - Drop all code related to ansible-playbook")
mocha and smart_proxy are already specified in Gemfile and others aren't
used at all.
@ekohl
Copy link
Member Author

ekohl commented Nov 11, 2024

Looks like Ruby 3.0 does some globbing that others don't. Mocking is such an awful tool.

.github/workflows/test.yml Outdated Show resolved Hide resolved
@ekohl
Copy link
Member Author

ekohl commented Nov 11, 2024

This time Ruby 3.1 failed. I suppose it's sort of random and suggests the test suite is already unreliable. The bigger matrix just uncovers it.

@@ -3,8 +3,7 @@ source 'https://rubygems.org'
gemspec

group :development do
gem 'smart_proxy', git: 'https://github.com/theforeman/smart-proxy',
branch: 'develop'
gem 'smart_proxy', github: 'theforeman/smart-proxy', branch: ENV['SMART_PROXY_BRANCH']
Copy link
Member

Choose a reason for hiding this comment

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

In theforeman/smart_proxy_plugin_template@71bd2b6 we used ENV.fetch(…, 'develop') -- or will branch default to the default branch if it's nil?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tested it on Fedora without specifying it and that works. The Bundler docs haven't been updated, but looks like this is a Rubygem 2.3 feature: rubygems/rubygems@6e032a9

Would you prefer to support < 2.3?

Copy link
Member

Choose a reason for hiding this comment

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

I have no idea what < 2.3 means for rubygems 🙈
But also, we should be consistent between here and the docs in the actions repo and the template ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

So I think we're not ready for that and I'll change it back to being explicit.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for checking!

This makes a change to always use glob_path in the runtime code so it
can be easily stubbed in tests. Stubbing Dir.glob creates problems if
other code also runs it.

It also makes the tests more explicit by providing a correct path and
asserting the resulting parsed result is correct. Previously it only
returned filenames, which isn't how Dir.glob works.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants