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

Ruby / Rubocop bump #71

Open
wants to merge 20 commits into
base: main
Choose a base branch
from
Open

Ruby / Rubocop bump #71

wants to merge 20 commits into from

Conversation

luke-hill
Copy link
Contributor

Bump minimum ruby version 2 more majors so we're finally up to date and fix up around half the problematic cops

🤔 What's changed?

⚡️ What's your motivation?

🏷️ What kind of change is this?

  • 🏦 Refactoring/debt/DX (improvement to code design, tooling, etc. without changing behaviour)

♻️ Anything particular you want feedback on?

📋 Checklist:

  • I agree to respect and uphold the Cucumber Community Code of Conduct
  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

This text was originally generated from a template, then edited by hand. You can modify the template here.

@luke-hill
Copy link
Contributor Author

Is Aruba complaining here because of the ruby warnings @mvz - Is there some way we can handle this short of me adding in the requisite gems now?

@mvz
Copy link

mvz commented Sep 26, 2024

I don't think Aruba can really filter this out without adding some very specific code.

Adding the requisite gems to the development dependencies is probably your best bet.

@luke-hill
Copy link
Contributor Author

Sorry @mvz - What I'm asking is, is the reason for the failures here due to deprecations / warnings, or for some other reason. Is there an easier way to get debug output from this to ascertain as checking the logs here doesn't make it easy

@mvz
Copy link

mvz commented Nov 19, 2024

@luke-hill I agree the failure messages are pretty bad. The ones were you get some sort of diff tell me the failures are indeed due to deprecation warnings:

-(output string is eq: "Feature: High strung\n\n  Scenario: Wired\n    Given we're all wired\n\n1 scenario (1 pending)\n1 step (1 pending)")
+Feature: High strung
+
+  Scenario: Wired
+    Given we're all wired
+
+1 scenario (1 pending)
+1 step (1 pending)
+/home/runner/work/cucumber-ruby-wire/cucumber-ruby-wire/vendor/bundle/ruby/3.3.0/gems/sys-uname-1.3.0/lib/sys/uname.rb:18: warning: ostruct was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0.
+You can add ostruct to your Gemfile or gemspec to silence this warning.
+/home/runner/work/cucumber-ruby-wire/cucumber-ruby-wire/vendor/bundle/ruby/3.3.0/bin/cucumber:25: warning: logger was loaded from the standard library, but will no longer be part of the default gems starting from Ruby 3.5.0.
+You can add logger to your Gemfile or gemspec to silence this warning.

The diff is bad because it doesn't compare with the expected string. However, you can see that the expected string starts ends with 1 step (1 pending), and the actual output has the warning about ostruct right after that.

I'll take a look at why the failure message are so bad.

Finally, Ruby 3.3.6 no longer emits these deprecation messages, so I think re-running the failing jobs will make them pass.

@mvz
Copy link

mvz commented Nov 19, 2024

@luke-hill I made a bug report for aruba here: cucumber/aruba#942

@mvz
Copy link

mvz commented Nov 25, 2024

@luke-hill I improved the failure message (see cucumber/aruba#943), and this will be released in aruba 2.3.0. However I need to update the release process to use the new(ish) automation so it could be a while.

@luke-hill
Copy link
Contributor Author

Thanks for the work! appreciated. I know what i need to fix here for now at least.

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.

2 participants