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

fix: Refactor C# mutation orchestration to a simpler design #2831

Merged
merged 36 commits into from
Mar 1, 2024

Conversation

dupdob
Copy link
Member

@dupdob dupdob commented Jan 16, 2024

Redesigned MutationStore and MutationContext classes to a simpler and more consistent design.

Redesigned function/method/accessor/lambda orchestrator via a common parent class. Removed all xxxExpressionToBodyEngine classes.

Fixed following (non reported) bugs:

  1. LocalFunction orchestrator may generate invalid local functions by adding erroneous return to void method (can be seen in the old unit test 🤦 )
  2. Improved return default injection policy. Those are injected only when:
    • function/method/accessor/lambda in body form
    • of non void (including async Task) return type
    • with at least one return present in the block
    • not ending with a return statement
    • not an iterator function (yield modifiers present)
  3. Add proper support for async local functions (not supported before)
  4. More consistent handling across methods/functions/accessors...
  5. provide support document explaining the purpose of each class used during CSharpOrchestration.

Many classes and lines of code have been removed, and hundreds of documentation lines have been added.

I must say I am quite happy with this one, borderline on proud.
😆

EDIT: obviously the associated documentation refers to this design and not the existing one

@dupdob dupdob force-pushed the refactor_orchestration branch from 363925c to 95acd20 Compare January 21, 2024 22:24
@dupdob dupdob force-pushed the refactor_orchestration branch from 95acd20 to adada5b Compare January 22, 2024 15:45
@dupdob dupdob force-pushed the refactor_orchestration branch from 081ea47 to ba1e510 Compare January 28, 2024 14:30
@dupdob dupdob force-pushed the refactor_orchestration branch from ba1e510 to f7a877e Compare January 29, 2024 09:24
@dupdob dupdob force-pushed the refactor_orchestration branch from fc03498 to 014af1f Compare February 2, 2024 16:45
@dupdob dupdob force-pushed the refactor_orchestration branch from 629a8c7 to 8ae782d Compare February 3, 2024 15:29
@dupdob dupdob force-pushed the refactor_orchestration branch from 8ae782d to 38b3f3b Compare February 3, 2024 15:31
@dupdob dupdob marked this pull request as ready for review February 3, 2024 15:33
Copy link
Contributor

@JAKeijzer96 JAKeijzer96 left a comment

Choose a reason for hiding this comment

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

I reviewed the technical reference doc. Comments are purely regarding grammar/formatting as I have no real knowledge of the Stryker internals (yet..)
In general it's an absolutely amazing write-up which is great for someone who's new to the project such as me. Thanks!

docs/technical-reference/Mutation Orchestration Design.md Outdated Show resolved Hide resolved
docs/technical-reference/Mutation Orchestration Design.md Outdated Show resolved Hide resolved
docs/technical-reference/Mutation Orchestration Design.md Outdated Show resolved Hide resolved
docs/technical-reference/Mutation Orchestration Design.md Outdated Show resolved Hide resolved
docs/technical-reference/Mutation Orchestration Design.md Outdated Show resolved Hide resolved
docs/technical-reference/Mutation Orchestration Design.md Outdated Show resolved Hide resolved
docs/technical-reference/Mutation Orchestration Design.md Outdated Show resolved Hide resolved
docs/technical-reference/Mutation Orchestration Design.md Outdated Show resolved Hide resolved
docs/technical-reference/Mutation Orchestration Design.md Outdated Show resolved Hide resolved
docs/technical-reference/Mutation Orchestration Design.md Outdated Show resolved Hide resolved
Thanks for the amazing editing work!! And sorry for these awful errors and typos

Co-authored-by: Jasper Keijzer <[email protected]>
@dupdob
Copy link
Member Author

dupdob commented Feb 7, 2024

@JAKeijzer96 : many thanks for the amazing editorial work here!!.
Btw, this document applies to the version in this PR. My initial objective was to write this doc, but I realized the design needed rework when writing it. Simple rule is: if it is hard to document, it can not stay as is.

Copy link
Member

@richardwerkman richardwerkman left a comment

Choose a reason for hiding this comment

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

@dupdob Thanks a lot for this improvement! Mainly the documentation is a real lifesaver, I understand what's going on a lot better now. Some small remarks here and there but I agree with the changes in general

@dupdob
Copy link
Member Author

dupdob commented Mar 1, 2024

@richardwerkman : your feedback is most welcomed. And thanks for the taking the time to read all this. This PR is probably no picnic to review

@dupdob dupdob force-pushed the refactor_orchestration branch from 8ae2fba to f78a179 Compare March 1, 2024 15:32
Copy link

sonarqubecloud bot commented Mar 1, 2024

@rouke-broersma rouke-broersma changed the title Refactor C# mutation orchestration to a simpler design fix: Refactor C# mutation orchestration to a simpler design Mar 1, 2024
@rouke-broersma rouke-broersma merged commit 02c8258 into master Mar 1, 2024
9 checks passed
@rouke-broersma rouke-broersma deleted the refactor_orchestration branch March 1, 2024 20:40
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.

4 participants