Skip to content

Fixes #39315 - Change rake task 'verbose' parameter#938

Merged
adamruzicka merged 1 commit into
theforeman:developfrom
ATIX-AG:fix-tests-upstream
May 13, 2026
Merged

Fixes #39315 - Change rake task 'verbose' parameter#938
adamruzicka merged 1 commit into
theforeman:developfrom
ATIX-AG:fix-tests-upstream

Conversation

@nadjaheitmann
Copy link
Copy Markdown
Contributor

The verbose option does not work anymore, so we have to explicitly pass the '--verbose' option now.

@nadjaheitmann nadjaheitmann marked this pull request as draft April 29, 2026 07:23
@nadjaheitmann nadjaheitmann marked this pull request as ready for review April 29, 2026 07:38
@adamruzicka
Copy link
Copy Markdown
Contributor

Before

# bundle exec rake test TEST="test/util_test.rb"
/Users/aruzicka/.local/share/rv/rubies/ruby-3.3.10/bin/ruby -w -I"lib:.:lib:test" -W1 /Users/aruzicka/.local/share/rv/gems/ruby/3.3.0/gems/rake-13.3.0/lib/rake/rake_test_loader.rb "test/util_test.rb"
/Users/aruzicka/vcs/foreman/smart-proxy/lib/proxy/log.rb:2: warning: syslog was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.4.0.
You can add syslog to your Gemfile or gemspec to silence this warning.
Loaded suite /Users/aruzicka/.local/share/rv/gems/ruby/3.3.0/gems/rake-13.3.0/lib/rake/rake_test_loader
Started
Finished in 0.171441 seconds.
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
12 tests, 15 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
69.99 tests/s, 87.49 assertions/s

After

bundle exec rake test TEST="test/util_test.rb"
/Users/aruzicka/vcs/foreman/smart-proxy/lib/proxy/log.rb:2: warning: syslog was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.4.0.
You can add syslog to your Gemfile or gemspec to silence this warning.
Loaded suite /Users/aruzicka/.local/share/rv/gems/ruby/3.3.0/gems/rake-13.3.0/lib/rake/rake_test_loader
Started
ProxyUtilTest:
  test_commandtask_as_array_with_exit_0:                                                                                                                                                                                                                        .: (0.042590)
  test_commandtask_with_exit_0:                                                                                                                                                                                                                                 .: (0.037225)
  test_commandtask_with_exit_1:                                                                                                                                                                                                                                 .: (0.035014)
  test_commandtask_with_input_and_exit_0:                                                                                                                                                                                                                       .: (0.037801)
  test_strict_encode64:                                                                                                                                                                                                                                         .: (0.000079)
  test_to_bool_default_true:                                                                                                                                                                                                                                    .: (0.000046)
  test_to_bool_empty:                                                                                                                                                                                                                                           .: (0.000028)
  test_to_bool_false_bool:                                                                                                                                                                                                                                      .: (0.000032)
  test_to_bool_true:                                                                                                                                                                                                                                            .: (0.000026)
  test_to_bool_true_bool:                                                                                                                                                                                                                                       .: (0.000023)
  test_util_shell_escape:                                                                                                                                                                                                                                       .: (0.000125)
  test_util_should_support_path:                                                                                                                                                                                                                                .: (0.000022)

Finished in 0.153628 seconds.
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
12 tests, 15 assertions, 0 failures, 0 errors, 0 pendings, 0 omissions, 0 notifications
100% passed
---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
78.11 tests/s, 97.64 assertions/s

Well it does change things, but according to the source it shouldn't be necessary? Have you found out why it doesn't work anymore?

@nadjaheitmann
Copy link
Copy Markdown
Contributor Author

Well it does change things, but according to the source it shouldn't be necessary? Have you found out why it doesn't work anymore?

I have relied on the bug explanation tbh: https://projects.theforeman.org/issues/39248

While this PR sure does not redesign or modernize the test suite, it at least unblocks the failing CI tests.

@m-bucher
Copy link
Copy Markdown

Well it does change things, but according to the source it shouldn't be necessary? Have you found out why it doesn't work anymore?

Something along the way seems to interpret the -v as a --version parameter. Not sure what that might be though.

Looking at the Before and After output, it seems to me that the t.verbose (which adds -v to the parameters) never really worked, because the Before output looks not-verbose.
The root cause might be in the plumbing of the rake-tasks unless it is a problem in rake itself 😅

IMHO this PR fixes the pipeline for now so 👍
😁

@ekohl
Copy link
Copy Markdown
Member

ekohl commented Apr 29, 2026

@nadjaheitmann which rake version are you using? ruby/rake@5886caa fixed a regression in Rake 13.4.0/13.4.1.

@nadjaheitmann
Copy link
Copy Markdown
Contributor Author

@nadjaheitmann which rake version are you using? ruby/rake@5886caa fixed a regression in Rake 13.4.0/13.4.1.

In this run, we have the pipeline failing and use rake 13.4.2: https://github.com/theforeman/smart-proxy/actions/runs/25096463025/job/73534458532#step:5:243

I started that run on this branch but with the fix commit reverted to double-check the issue was actually solved by the fix.

@nadjaheitmann
Copy link
Copy Markdown
Contributor Author

Reading through the issue description, @ogajduse proposed a fix directly in ci-reporter/ci_reporter_test_unit#13 which is closer to the root cause of the problem than this PR which is more targeted at resolving the symptoms. But it is very unlikely to be merged given the project has not received any changes during the last 10 years.

@nadjaheitmann
Copy link
Copy Markdown
Contributor Author

@adamruzicka @ekohl I can see that pipelines are still broken. Should this be merged as a temporary solution for the pipeline to turn green or is there some other effort coming in soonish, that will supersede this PR?

@ofedoren ofedoren mentioned this pull request May 12, 2026
@ekohl
Copy link
Copy Markdown
Member

ekohl commented May 12, 2026

Not blocking this merge (since it's easy to cherry pick), but longer term we should evaluate migrating from test-unit to minitest which aligns with foreman. Having a common testing framework is IMHO a good thing. We previously discussed that briefly on Matrix, but that gets lost easily so noting it here.

@ogajduse
Copy link
Copy Markdown
Member

Not blocking this merge (since it's easy to cherry pick), but longer term we should evaluate migrating from test-unit to minitest which aligns with foreman. Having a common testing framework is IMHO a good thing. We previously discussed that briefly on Matrix, but that gets lost easily so noting it here.

In that case, I'd recommend creating a new Redmine issue as the one it's currently linked to aims to implement the long-term solution.

@nadjaheitmann
Copy link
Copy Markdown
Contributor Author

nadjaheitmann commented May 12, 2026

In that case, I'd recommend creating a new Redmine issue as the one it's currently linked to aims to implement the long-term solution.

Good enough if I change 'Fixes' to 'Refs'? It literally fixes what is written in the title :) smart-proxy test failures due to rake 13.4.2 breaking ci_reporter_test_unit

@adamruzicka
Copy link
Copy Markdown
Contributor

It literally fixes what is written in the title

But then the text proposes a completely different solution which leaves it in a weird place.

Good enough if I change 'Fixes' to 'Refs'?

I'm not sure if it wouldn't confuse prprocessor. I'd opt for a separate issue, even if just to stay on the safe side.

The verbose option does not work anymore, so we have to explicitly pass
the '--verbose' option now.
@nadjaheitmann
Copy link
Copy Markdown
Contributor Author

nadjaheitmann commented May 13, 2026

But then the text proposes a completely different solution which leaves it in a weird place.

I created a new issue and just copied over part of the issue description. IMO, it makes sense to change the title of https://projects.theforeman.org/issues/39248 to something more fitting, e.g. Modernize smart-proxy test suite to make clear what the purpose of the issue is.

I'm not sure if it wouldn't confuse prprocessor. I'd opt for a separate issue, even if just to stay on the safe side.

I'd assume it would be fine :) Now, we might need to remove the PR from the 'old' issue manually. I don't think it is updated automatically, is it?

Edit: I was able to delete the PR from issue 39248 .

@ofedoren ofedoren changed the title Change rake task 'verbose' parameter Fixes #39315 - Change rake task 'verbose' parameter May 13, 2026
@adamruzicka adamruzicka merged commit 4a468d7 into theforeman:develop May 13, 2026
10 checks passed
@adamruzicka
Copy link
Copy Markdown
Contributor

Thank you @nadjaheitmann !

@nadjaheitmann nadjaheitmann deleted the fix-tests-upstream branch May 13, 2026 09:45
@ogajduse
Copy link
Copy Markdown
Member

Backported to 3.19-stable in #942

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants