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 foreman GH action workflow #923

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Comment on lines -17 to -20
Copy link
Member

Choose a reason for hiding this comment

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

These are now only pulled in via released gems, no longer the git version. If you care about this, I'd recommend adding it to your Gemfile. If you want, you can limit it to ENV['GITHUB_ACTIONS'] == 'true'.

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
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
Copy link
Member

Choose a reason for hiding this comment

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

We install RuboCop testing without development dependencies, but with test dependencies. So just moving everything to a development dependency broke the RuboCop.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah so these all would need to be done as:
gem.add_dependency then since I was looking at the docs for gemspec and you can't do grouping like you could in the Gemfile

If that is correct, I will go ahead and get it changed up

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, in a gemspec you can only do add_dependency (which is runtime) and add_development_dependency which is devel and maps to the development bundler group from the gemfile

Copy link
Member

@evgeni evgeni Jan 25, 2024

Choose a reason for hiding this comment

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

fwiw, @ekohl is wrong, we do not skip development group when installing rubocop: https://github.com/theforeman/actions/blob/v0/.github/workflows/rubocop.yml

we do that only in the plugin flow: https://github.com/theforeman/actions/blob/v0/.github/workflows/foreman_plugin.yml#L45

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
9 changes: 5 additions & 4 deletions hammer_cli_katello.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -63,18 +63,19 @@ Gem::Specification.new do |gem|
gem.name = 'hammer_cli_katello'
gem.require_paths = ['lib']
gem.version = HammerCLIKatello.version
gem.required_ruby_version = '>= 2.7'

gem.add_dependency 'hammer_cli_foreman'
gem.add_dependency 'hammer_cli_foreman_tasks'
gem.add_dependency 'gettext', '>= 3.1.3', '< 4.0.0'
Copy link
Member

Choose a reason for hiding this comment

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

this is a devel dependency I think?


gem.add_development_dependency 'rake'
gem.add_development_dependency 'thor'
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 'ci_reporter', '>= 1.6.3', "< 2.0.0"
gem.add_development_dependency 'rubocop', '0.42'
Copy link
Member

Choose a reason for hiding this comment

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

This is a seriously old version. It wouldn't surprise me if this wouldn't work on Ruby 3.

gem.add_development_dependency 'rubocop-checkstyle_formatter'
Copy link
Member

Choose a reason for hiding this comment

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

This was only used for testing in Jenkins, but even that doesn't need it anymore.

Suggested change
gem.add_development_dependency 'rubocop-checkstyle_formatter'

end
Loading