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

Migrate to GitHub Actions from CircleCI #398

Merged
merged 1 commit into from
Nov 29, 2023

Conversation

koic
Copy link
Member

@koic koic commented Nov 27, 2023

This PR will centralize CI to GitHub Actions as introduced in #388.


Before submitting the PR make sure the following are checked:

  • The PR relates to only one subject with a clear title and description in grammatically correct, complete sentences.
  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Ran bundle exec rake default. It executes all tests and runs RuboCop on its own code.
  • Added an entry (file) to the changelog folder named {change_type}_{change_description}.md if the new code introduces user-observable changes. See changelog entry format for details.

@koic koic force-pushed the migrate_to_gha_from_circleci branch 5 times, most recently from abf745d to 9a1296e Compare November 29, 2023 06:01
This PR will centralize CI to GitHub Actions as introduced in
rubocop#388.
@koic koic force-pushed the migrate_to_gha_from_circleci branch from 9a1296e to eef0431 Compare November 29, 2023 15:33
@koic koic merged commit efa8759 into rubocop:master Nov 29, 2023
12 checks passed
@koic koic deleted the migrate_to_gha_from_circleci branch November 29, 2023 15:40
@Earlopain Earlopain mentioned this pull request May 29, 2024
@Earlopain
Copy link
Contributor

Earlopain commented Aug 21, 2024

Hey @koic, locally I'm getting the failures you excluded for JRuby with CRuby, but only when running the full spec suite with bundle exec rspec or bunde exec rake (seed is 26805 if thats important). If I run the file in isolation with bundle exec rspec spec/rubocop/cop/performance/sum_spec.rb tests pass. Do you encounter the same thing? Maybe there is some state leak between tests and JRuby just happened to hit it here.

I wonder if this is something I should be looking into, but if its only me I wouldn't bother. I regenerated my lockfile but I can't say if that is related to that.

@koic
Copy link
Member Author

koic commented Aug 21, 2024

Hm, I couldn't reproduce it on my local environment. It also passes when using the seed.

@Earlopain
Copy link
Contributor

I think I figured it out. Not sure why it's not failing for you though. Will open a PR.

@Earlopain
Copy link
Contributor

Well, not really. I bisected it down to the following order bundle exec rspec ./spec/rubocop/cli/autocorrect_spec.rb:16 ./spec/rubocop/cop/performance/sum_spec.rb:5 and the culprid seemed to be overriding the default config here without resetting it:

RuboCop::ConfigLoader.default_configuration = nil
RuboCop::ConfigLoader.default_configuration.for_all_cops['SuggestExtensions'] = false
RuboCop::ConfigLoader.default_configuration.for_all_cops['NewCops'] = 'disable'
(how incredibly cool is rspec --bisect, TIL)

Solution seemed simple enough by changing it into this:

  around do |ex|
    previous_config = RuboCop::ConfigLoader.default_configuration

    RuboCop::ConfigLoader.default_configuration = nil
    RuboCop::ConfigLoader.default_configuration.for_all_cops['SuggestExtensions'] = false
    RuboCop::ConfigLoader.default_configuration.for_all_cops['NewCops'] = 'disable'
    ex.call
  ensure
    RuboCop::ConfigLoader.default_configuration = previous_config
  end

which did remove the failures. But now I'm not getting failures anymore at all even with the command that previously reliably produced them without changes to the code, and other specs in rubocop also do the same pattern without problems. So I'm a bit stumped again 🤷

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