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.

Sign up for GitHub

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

Jump to bottom

Use foreman GH action workflow #924

Merged
merged 4 commits into from
Jan 25, 2024
Merged

Use foreman GH action workflow #924

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f1d41e2
Use foreman GH action workflow
chris1984 Jan 22, 2024
94424c3
define versions for all dependencies
evgeni Jan 25, 2024
f66bd75
pin rake to 10.x as newer make the tests fail
evgeni Jan 25, 2024
61e7a1c
drop checkstyle_formatter
evgeni Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 14 additions & 23 deletions .github/workflows/unit-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,27 +1,18 @@
name: Run unit tests
name: CI

on:
- push
- pull_request
on: pull_request

jobs:
test:
concurrency:
group: ${{ github.ref_name }}-${{ github.workflow }}
cancel-in-progress: true

runs-on: ubuntu-latest
strategy:
matrix:
ruby: ["2.7"]
jobs:
rubocop:
name: Rubocop
uses: theforeman/actions/.github/workflows/rubocop.yml@v0

steps:
- uses: actions/checkout@v2
- name: Add hammer-cli-foreman to local gem file
run: echo "gem 'hammer_cli_foreman', :git => 'https://github.com/theforeman/hammer-cli-foreman.git'" > Gemfile.local
- name: Add hammer-cli to local gem file
run: echo "gem 'hammer_cli', :git => 'https://github.com/theforeman/hammer-cli.git'" >> Gemfile.local
- name: Set up Ruby
uses: ruby/setup-ruby@v1
with:
ruby-version: ${{ matrix.ruby }}
bundler-cache: true
- name: Run tests
run: bundle exec rake
test:
name: Tests
uses: theforeman/actions/.github/workflows/test-gem.yml@v0
with:
command: bundle exec rake test
15 changes: 0 additions & 15 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,6 @@ source "https://rubygems.org"

gemspec

# for generating i18n files, gettext > 3.0 dropped ruby 1.8 support
gem 'gettext', '>= 3.1.3', '< 4.0.0'

group :test do
gem 'rake', '~> 10.1.0'
gem 'thor'
gem 'minitest', '4.7.4'
gem 'minitest-spec-context'
gem 'mocha'
gem 'coveralls', '0.8.23', require: false
gem 'ci_reporter', '>= 1.6.3', "< 2.0.0", :require => false
gem 'rubocop', '0.42'
gem 'rubocop-checkstyle_formatter'
end

# load local gemfile
['Gemfile.local.rb', 'Gemfile.local'].map do |file_name|
local_gemfile = File.join(File.dirname(__FILE__), file_name)
Expand Down
8 changes: 0 additions & 8 deletions Rakefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,6 @@ end
begin
require 'rubocop/rake_task'
RuboCop::RakeTask.new

desc "Runs Rubocop style checker with xml output for Jenkins"
task 'rubocop:jenkins' do
system("bundle exec rubocop \
--require rubocop/formatter/checkstyle_formatter \
--format Rubocop::Formatter::CheckstyleFormatter \
--no-color --out rubocop.xml")
end
rescue => _
puts "Rubocop not loaded."
end
Expand Down
21 changes: 10 additions & 11 deletions hammer_cli_katello.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,17 @@ Gem::Specification.new do |gem|
gem.name = 'hammer_cli_katello'
gem.require_paths = ['lib']
gem.version = HammerCLIKatello.version
gem.required_ruby_version = '>= 2.7', '< 3.2'

gem.add_dependency 'hammer_cli_foreman'
gem.add_dependency 'hammer_cli_foreman_tasks'
gem.add_dependency 'hammer_cli_foreman', '~> 3.9'
gem.add_dependency 'hammer_cli_foreman_tasks', '~> 0.0.20'

gem.add_development_dependency 'rake'
gem.add_development_dependency 'thor'
gem.add_development_dependency 'ci_reporter', '>= 1.6.3', "< 2.0.0"
gem.add_development_dependency 'gettext', '>= 3.1.3', '< 4.0.0'
gem.add_development_dependency 'minitest', '4.7.4'
gem.add_development_dependency 'minitest-spec-context'
gem.add_development_dependency 'simplecov'
gem.add_development_dependency 'mocha'
gem.add_development_dependency 'ci_reporter'

gem.add_development_dependency "rubocop", "0.42"
gem.add_development_dependency "rubocop-checkstyle_formatter"
gem.add_development_dependency 'minitest-spec-context', '~> 0.0.5'
gem.add_development_dependency 'mocha', '~> 2.0'
gem.add_development_dependency 'rake', '~> 10.0'
Copy link
Member

Choose a reason for hiding this comment

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

I think this broke on Jenkins:

21:27:43  /usr/local/rvm/gems/ruby-2.7.4@hammer-cli-katello-master-source-release-193-1-ruby-2-7/gems/bundler-2.4.22/lib/bundler/rubygems_integration.rb:308:in `block in replace_bin_path': can't find executable rake for gem rake. rake is not currently included in the bundle, perhaps you meant to add it to your Gemfile? (Gem::Exception)

Couldn't you have bumped to '~> 13.0`?

Copy link
Member Author

Choose a reason for hiding this comment

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

interesting. and no, see f66bd75

Copy link
Member Author

Choose a reason for hiding this comment

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

11+ does something that our tests start failing

Copy link
Member

Choose a reason for hiding this comment

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

Regardless, Jenkins still doesn't install the development dependencies so a test group is still useful. Or just list rake unconditionally in Gemfile.

Copy link
Member

Choose a reason for hiding this comment

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

11+ does something that our tests start failing

Yes, it starts displaying warnings.

I couldn't find the source of the incorrect interpolation warning, which certainly is a bug. I could trace it to https://github.com/theforeman/hammer-cli/blob/f38afde3e44667908ed39078c48fb26a2244aa8b/lib/hammer_cli/utils.rb#L15 but didn't find a stack trace of where it's called from. It appears (at least on Ruby 3.2) to end up calling string % nil which emits a warning.

I at least opened #925 for some other bugs I noticed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just to mention this here as well: I'd really try to use either unpinned rake or at least as Ewoud suggested ~> 13.0. Hammer core does this and it seems to be working for quite some time?

Copy link
Member

Choose a reason for hiding this comment

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

This gem raises many warnings, mostly about redefining environment, but also at least one string formatting warning.

Copy link
Member

Choose a reason for hiding this comment

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

I will bump it to ~> 13.0 as Ewoud suggested and see what else we can drop or bump from here

Copy link
Member

Choose a reason for hiding this comment

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

Regardless, Jenkins still doesn't install the development dependencies so a test group is still useful. Or just list rake unconditionally in Gemfile.

This is still true and nightlies for this package have been broken since. Is there still a desire for nightly packages?

gem.add_development_dependency 'rubocop', '0.42'
gem.add_development_dependency 'thor', '~> 1.0'
end
Loading