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

Introduce matrix for unit tests. #19

Conversation

sairamsrinivasan
Copy link
Collaborator

Changes -

  • Set up matrix in CI to run unit tests
  • Add exclusion rules for incompatible versions

reduce test matrix

remove mongoid-head gemfile as it is no longer used

reduce matrix entry to multi dimension matrix

test include for mongoid 9

exclude incompatible versions

remove compatible configurations

remove minor version specification to reduce number of combinations

reduce the number of parallel jobs and incorporate caching

remove caching

add caching

create single entry

add matrix with single entry

add ruby 3.0 and mongoid 8

complete string

add exclusion rules

add more exclusion rules

correct gem file name and add more exclusion rules

add more exclusion rules for mongoid_3

add exlcusion rules

add ruby 2.7 and mongoid 4

add more exclusion rules for ruby 3.7

get rid of mongodb 3.6

add ruby 2.7 and mongoid 3 and 4 to exlcusion list

add ruby 2.7 and mongoid 5

exclude ruby 2.6 and mongoid 3 and allow ruby 2.6 and mongoid 7 to run

add matrix for unit tests

modify changelog
@sairamsrinivasan
Copy link
Collaborator Author

@dblock - Can you please review when you get the chance? Thanks!

@dblock
Copy link
Contributor

dblock commented Jul 7, 2024

I am open to this, but why is it better? Looks like we're now maintaining a long list of excludes instead of just saying what we want to run.

@sairamsrinivasan
Copy link
Collaborator Author

sairamsrinivasan commented Jul 7, 2024

I am open to this, but why is it better? Looks like we're now maintaining a long list of excludes instead of just saying what we want to run.

My understanding is that the matrix allows us to define combinations of dependencies that may be compatible with each other that we can easily test. Manually adding a combination of gems to an even longer list can be difficult to manage. The caveat is that some gem versions can be incompatible with others, which can be easily identified by a failing build pipeline and excluded in subsequent runs.

Another alternative that I'd like to propose is that we trim down the list of supported versions that we need to use for the test. Mongoid's documentation page only supports notes for v7.3+ - which is already more than 3 years old. I have to dig up the release notes on github to find out more about older versions.

Can we test only the following versions:

MongoDB 5+
Ruby 2.7+
Mongoid 7+?

What are your thoughts about this?

@dblock
Copy link
Contributor

dblock commented Jul 7, 2024

I would prefer to leave the existing definition rather than exclusions. It's very confusing to me what is actually run with the matrix-like definition.

On the versions I only deprecate things if that's actually useful. For example, if additional work is required to maintain ruby 2.6 compatibility or code can be deleted if we cut it, go for it. But just because mongoid 6 is old doesn't mean we should be causing users pain. I interacted with a project using mongoid 6 as recently as this week.

Always open to be convinced otherwise! Please do insist if you think something is worth it. And thanks so much for your contributions and work here, I sincerely appreciate it.

@sairamsrinivasan
Copy link
Collaborator Author

I would prefer to leave the existing definition rather than exclusions. It's very confusing to me what is actually run with the matrix-like definition.

On the versions I only deprecate things if that's actually useful. For example, if additional work is required to maintain ruby 2.6 compatibility or code can be deleted if we cut it, go for it. But just because mongoid 6 is old doesn't mean we should be causing users pain. I interacted with a project using mongoid 6 as recently as this week.

Always open to be convinced otherwise! Please do insist if you think something is worth it. And thanks so much for your contributions and work here, I sincerely appreciate it.

@dblock - Thank you for taking the time to review this PR and offer feedback. Let me think about this further to see how we can improve the existing definitions. I'm closing this PR for now.

I opened another PR that adds dependency caching. You can find the changes here: #20

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