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

Takeover of PR #388 (Scope NUnit parallelization to generated classes instead of assembly-level) #432

Merged
merged 8 commits into from
Feb 7, 2025

Conversation

gasparnagy
Copy link
Contributor

@gasparnagy gasparnagy commented Feb 5, 2025

🤔 What's changed?

Taken over from #388, see details there

  • Removed assembly-wide attribute declaring parallelization to be InstancePerTestCase (not the NUnit default)
  • Add the attribute to the generated classes.

⚡️ What's your motivation?

Fixes #379

🏷️ What kind of change is this?

  • 🐛 Bug fix (non-breaking change which fixes a defect)

♻️ Anything particular you want feedback on?

📋 Checklist:

  • I've changed the behaviour of the code
    • I have added/updated tests to cover my changes.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly.
  • Users should know about my change
    • I have added an entry to the "[vNext]" section of the CHANGELOG, linking to this pull request & included my GitHub handle to the release contributors list.

This text was originally taken from the template of the Cucumber project, then edited by hand. You can modify the template here.

@gasparnagy gasparnagy requested a review from obligaron February 5, 2025 14:58
@gasparnagy gasparnagy marked this pull request as ready for review February 5, 2025 14:59
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we replace the use of these custom functions with the more idiomatic form of
CodeTypeReference?

Example: new CodeTypeReference(typeof(Io.Cucumber.Messages.Types.Source), CodeTypeReferenceOptions.GlobalReference);

I've not verified that this works properly for VB generation but I would presume that it does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh. That's a good hint. I didn't know that that exists. We can try it in a new PR...

Copy link
Contributor

Choose a reason for hiding this comment

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

I've not verified that this works properly for VB generation but I would presume that it does.

Added a test to Messages for code gen using VB and this form of the CodeTypeReference api works as expected.
We should switch over to it as the custom function we have today for globalizing type names doesn't support VB.

I volunteer to draft the PR, but can wait until after this PR goes in.

@gasparnagy gasparnagy requested a review from obligaron February 6, 2025 16:19
@gasparnagy gasparnagy merged commit 60cab92 into main Feb 7, 2025
5 checks passed
@gasparnagy gasparnagy deleted the pr/n388_379_parallelization-default branch February 7, 2025 07:55
@TotalTest
Copy link

Hey @gasparnagy is there a timeframe of when this will be released?

@dqwork
Copy link

dqwork commented Feb 10, 2025

On the original issue they said early 'next' (ie now this) week
#379 (comment)

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.

With NUnit 4.x, "Only static OneTimeSetUp and OneTimeTearDown are allowed for InstancePerTestCase mode"
6 participants