-
-
Notifications
You must be signed in to change notification settings - Fork 153
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
Support multiple subject expressions [RSpec] #1125
Conversation
938611b
to
c37d268
Compare
CI doesn't seem to be generating other mutations either. Am I missing something here? |
@dgollahon I'm fine with your choices of overwriting. Code wise I'm also okay, I want to do some exploratory testing + verifying the mutation coverage self test you highlighted before merge. |
@dgollahon So very good observation that the rspec integration was not included. This is basically the case as mutant, at the time it runs subject matching, had not loaded the rspec integration. Applying this diff to your commit: diff --git a/lib/mutant/integration/rspec.rb b/lib/mutant/integration/rspec.rb
index fba19d4a..efc7b77e 100644
--- a/lib/mutant/integration/rspec.rb
+++ b/lib/mutant/integration/rspec.rb
@@ -102,7 +102,7 @@ module Mutant
expression = metadata.fetch(:mutant_expression)
expressions =
- expression.kind_of?(Array) ? expression : [expression]
+ expression.instance_of?(Array) ? expression : [expression]
expressions.map(&method(:parse_expression))
else
diff --git a/mutant.yml b/mutant.yml
index cdc17b0f..540e82f4 100644
--- a/mutant.yml
+++ b/mutant.yml
@@ -4,4 +4,5 @@ includes:
integration: rspec
requires:
- mutant
+- mutant/integration/rspec
- mutant/meta This makes the integration visible to mutant + covers the one mutation that was reported. |
- Allows mutant users to specify that a test covers multiple subjects using `mutant_expression`, not just one. - Closes #1079
c37d268
to
a161335
Compare
@dgollahon I've released this as v0.10.9. Cheers, thanks for the contribution. |
@@ -4,4 +4,5 @@ includes: | |||
integration: rspec | |||
requires: | |||
- mutant | |||
- mutant/integration/rspec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mbj Do we need to do the same thing for the minitest integration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dgollahon I'm not considering this the final solution. I do not want to load rspec and minitest together at this point.
For now I chose this route as mutant is specified via rspec, for minitest I want to use a different solution, likely a separate mutant config altogether.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, makes sense--just wanted to make sure that possible issue wasn't overlooked.
Thanks for the release! |
mutant_expression
, not just one.@mbj Is this what you had in mind in terms of implementation? I thought about concatenating the subject but then I'm not sure how you could override the default subject. I was thinking just an array of subjects would be a good starting point.
Additionally, I tried to mutate this locally but it didn't seem to pick up the subject with
--since
or a direct expression. I must be doing something wrong there--not sure if CI picked up on the changes or not.