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

Make sure tests use the same gem versions we use in production #8138

Merged
merged 5 commits into from
Jan 5, 2024
Merged

Make sure tests use the same gem versions we use in production #8138

merged 5 commits into from
Jan 5, 2024

Conversation

deivid-rodriguez
Copy link
Contributor

In #8134 it was noticed that running rubocop from an ecosystem directory, will run the latest version of RuboCop, not the ones we have locked at updater/Gemfile.lock.

This PR is an experiment to try to make sure a consistent version of rubocop is run all around while also simplifying our setup.

@deivid-rodriguez deivid-rodriguez requested a review from a team as a code owner October 4, 2023 05:55
@github-actions github-actions bot added L: php:composer Issues and code for Composer L: ruby:bundler RubyGems via bundler L: elixir:hex Elixir packages via hex L: java:gradle Maven packages via Gradle L: go:modules Golang modules L: github:actions GitHub Actions L: elm Elm packages L: git:submodules Git submodules L: terraform Terraform packages L: docker Docker containers L: rust:cargo Rust crates via cargo L: dotnet:nuget NuGet packages via nuget or dotnet L: java:maven Maven packages via Maven L: dart:pub Dart packages via pub L: javascript L: python L: swift Swift packages labels Oct 4, 2023
@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/improve-rubocop branch 6 times, most recently from 9a56a6a to 98e4c3c Compare October 4, 2023 16:31
Nishnha
Nishnha approved these changes Oct 4, 2023
View reviewed changes
Copy link
Member

@Nishnha Nishnha left a comment

Choose a reason for hiding this comment

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

These changes made a lot more sense after I saw your comment in #8134 (comment)

I think this will work!

@deivid-rodriguez
Copy link
Contributor Author

Thanks @Nishnha, yes, I think this will work.

However, turns out some specs depend on a Gemfile file being present at the root of an ecosystem. This seems unnecessary and confusing, and I've also been bitten by similar issues when running specs on my Linux laptop, but it requires some extra work to fix it.

@deivid-rodriguez deivid-rodriguez force-pushed the deivid-rodriguez/improve-rubocop branch 4 times, most recently from 5235c25 to cf4e095 Compare November 10, 2023 15:31
@deivid-rodriguez deivid-rodriguez mentioned this pull request Nov 16, 2023
Merged
@deivid-rodriguez deivid-rodriguez changed the title Improve RuboCop and development setup in general Make sure tests use the same gem versions we use in production Jan 4, 2024
@deivid-rodriguez
Copy link
Contributor Author

I think this is good to go now.

Doing this would prevent sudden introduction of broken tests by external releases, like it happened at #8509. That workaround will no longer be necessary with this PR, although I'm not reverting it, because we do use json directly, so it's a runtime dependency after all.

Also, note that in my opinion the best solution here would be for each ecosystem to have its own Gemfile & Gemfile.lock file, but without multidir version updates group PRs, it's going to get very noisy, so reusing updater/ lockfile seems good enough for now?

@deivid-rodriguez
Improve dependency_file specs
ca60acb 
They were relying on the presence of a Gemfile file in the folder where
they run. Switch to a folder that will always have a Gemfile.
@deivid-rodriguez
Move webrick pub development dependency to common
f78dd34 
And add it to the main Gemfile.

Yes, only pub depends on it, but it's the single case of an ecosystem
having a specific development dependency. Moving it to common will allow
fulling locking all dependencies in the updater lockfile.
@deivid-rodriguez
Use production versions of dependencies during tests
a79f296 
Before, each ecosystem would have a `Gemfile` but not a lockfile, so
tests would run against the latest compatible version of all
dependencies.

With this change, ecosystem specs will run using the Gemfile and
Gemfile.lock in updater/, so they will consistenly use the same versions
as production.
@deivid-rodriguez
All deps are now shared
e707dbb
@deivid-rodriguez
Simplify gitignores
1ba42ca 
We no longer gitignore any Gemfile.lock file.
@deivid-rodriguez
Copy link
Contributor Author

Let's see how this setup works!

@deivid-rodriguez deivid-rodriguez merged commit 8d2aecc into main Jan 5, 2024
116 checks passed
@deivid-rodriguez deivid-rodriguez deleted the deivid-rodriguez/improve-rubocop branch January 5, 2024 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Nishnha Nishnha Nishnha approved these changes

@jakecoffman jakecoffman jakecoffman approved these changes

Assignees
No one assigned
Labels
L: dart:pub Dart packages via pub L: docker Docker containers L: dotnet:nuget NuGet packages via nuget or dotnet L: elixir:hex Elixir packages via hex L: elm Elm packages L: git:submodules Git submodules L: github:actions GitHub Actions L: go:modules Golang modules L: java:gradle Maven packages via Gradle L: java:maven Maven packages via Maven L: javascript L: php:composer Issues and code for Composer L: python L: ruby:bundler RubyGems via bundler L: rust:cargo Rust crates via cargo L: swift Swift packages L: terraform Terraform packages
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@deivid-rodriguez @jakecoffman @Nishnha