Skip to content

Fix ParallelizableAttribute with TestFixtureSource - #4099

Merged
SimonCropp merged 2 commits into
nunit:masterfrom
jairbubbles:fix-parallelizableattribute-with-testfixturesource
Apr 8, 2023
Merged

SimonCropp merged 2 commits into
nunit:masterfrom
jairbubbles:fix-parallelizableattribute-with-testfixturesource

Conversation

@jairbubbles

@jairbubbles jairbubbles commented Apr 20, 2022 •

Copy link
Copy Markdown
Contributor

Tests declared like that:

[TestFixtureSource("Data")]
[Parallelizable(ParallelScope.All)]
public class TestFixtureSourceWithParallelizableAttribute

were no longer run in parallel since that change: 247ede8

Similar to #3715 I explicitely apply ParallelizableAttribute to children tests.

Closes #3735

NB: I couldn't run tests in master branch so I tested in the 3.0 dev branch.

@jontdelorme

Copy link
Copy Markdown

Hello, we are waiting for that fix to update to the latest version. Anyone ready for the review?

@jairbubbles

Copy link
Copy Markdown
Contributor Author

Hey @SimonCropp anything I can do to get this merged?

@SimonCropp

Copy link
Copy Markdown
Contributor

@jairbubbles LGTM. but i am not a maintainer on this project, so cant merge

@jairbubbles

Copy link
Copy Markdown
Contributor Author

Oh sorry I found your name in the last PRs merged but I guess you were just the author then.

@jairbubbles

Copy link
Copy Markdown
Contributor Author

(Thx @stevenaw for updating my branch)

@jairbubbles

Copy link
Copy Markdown
Contributor Author

Hi @stevenaw, anything I can do to help this PR getting merged?

@stevenaw

Copy link
Copy Markdown
Member

Hi @jairbubbles thanks for asking. Nothing on your end, this just needs a review by the team. Sorry it's taken so long to be looked at. I'd hoped to review it but I got a little unexpectedly busy myself. I still hope to get to this soon.

@OsirisTerje

Copy link
Copy Markdown
Member

@stevenaw This looks very simple. Do you think we can just merge it?

@stevenaw stevenaw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd be alright with that @OsirisTerje
This LGTM too, thanks @jairbubbles for your contribution!

@stevenaw

stevenaw commented Apr 7, 2023

Copy link
Copy Markdown
Member

I'm having a bit of a hard time merging.

@jairbubbles
jairbubbles force-pushed the fix-parallelizableattribute-with-testfixturesource branch from bac37e3 to ade5b0e Compare April 8, 2023 07:27
@jairbubbles

Copy link
Copy Markdown
Contributor Author

I rebased and resolved the conflict manually.

@SimonCropp SimonCropp added this to the 4.0 milestone Apr 8, 2023
@SimonCropp
SimonCropp merged commit d3ed922 into nunit:master Apr 8, 2023
@jairbubbles
jairbubbles deleted the fix-parallelizableattribute-with-testfixturesource branch April 8, 2023 12:58
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.

Parallelizable TestFixtureSource with TestFixtureData not running in parallel

5 participants