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

feat(Mutators): Add conditional operator mutator #2583

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

michalusio
Copy link

@michalusio michalusio commented Jun 21, 2023

Add conditional operator mutator
Add tests for conditional operator mutator
Include conditional operator mutator in mutations.md

For now the mutator has MutationLevel == Complete, as the docs said that the level will be decided by the team.

Closes #2581.

Michał Isalski added 2 commits June 21, 2023 22:38
Add conditional operator mutator
Add tests for conditional operator mutator
Include conditional operator mutator in mutations.md
@dupdob
Copy link
Member

dupdob commented Jun 22, 2023

sorry to break this to you, but Stryker does mutate the condition part of conditional expressions. May I suggest you clarify why the existing mutation is not sufficient and/or why it is not working on your code?

@michalusio
Copy link
Author

@dupdob As I said in the issue (#2581), the NegateConditionMutator creates a mutation that negates the condition.
It doesn't force it to true or false, which was the point of the issue, and of this mutator.

@michalusio michalusio marked this pull request as ready for review June 28, 2023 19:17
@dupdob
Copy link
Member

dupdob commented Jun 29, 2023

@michalusio : could you adjust end to end (called integration here) tests' expected results, so that the build can pass ?
you can see them failing here: https://dev.azure.com/stryker-mutator/Stryker/_build/results?buildId=15435&view=logs&jobId=59ee5473-4355-59e8-3cdb-05fe186645bf&j=59ee5473-4355-59e8-3cdb-05fe186645bf&t=0960e961-80fc-58db-6359-7743b41142d1
and the assertion logic is here: https://github.com/stryker-mutator/stryker-net/blob/master/integrationtest/ValidationProject/ValidateStrykerResults.cs . Just a small fix, and this gives you a chance for better understanding the Stryker's CI pipeline

@dupdob dupdob self-requested a review June 29, 2023 07:17
Copy link
Member

@dupdob dupdob left a comment

Choose a reason for hiding this comment

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

can you please answer how previously difficult cases are supported by the new mutator ?

@@ -61,19 +60,5 @@ public void MutatesStatementWithMethodCallWithNoArguments(string method)
mutation.ReplacementNode.ToString().ShouldBe("!(Method())");
mutation.DisplayName.ShouldBe("Negate expression");
}

[Theory]
[InlineData("var y = x is object result ? result.ToString() : null;")] // can't mutate inline var declaration
Copy link
Member

Choose a reason for hiding this comment

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

have you checked how this expression is mutated, and most importantly, does it compile?

Copy link
Member

@rouke-broersma rouke-broersma left a comment

Choose a reason for hiding this comment

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

As reviewed by @dupdob some changes and explanations are requested

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.

Add mutation for conditional (ternary) operator
3 participants