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

Add support for parallel sub-tests and remove suite.Suite pseudo-interitance #1109

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

Conversation

maroux
Copy link

@maroux maroux commented Aug 4, 2021

Summary

Add support for parallel sub-tests and remove suite.Suite pseudo-interitance

Changes

  • Run sub-tests in a group so TearDownSuite is called in the right order
  • Remove suite.Suite interface / struct because it's a bad attempt at
    inheritance which is causing problems.

Motivation

Currently, parallel sub-tests are broken with suite. Go runs parallel tests by deferring
them until the end of parent test. testing.T.Run returns early, and thus the deferred calls
in suite fire off, such as TearDownSuite (the test itself runs in a separate go-routine).

Additionally, since there's only one T object per suite instance, this means during parallel
tests, this object gets overridden by subsequent tests's F method, which leads to test output being associated with the wrong test,
and potentially other problems because testing.T is NOT concurrent-safe.

Example usage (if applicable)

type ExampleTestSuite struct {}

func (suite *ExampleTestSuite) TestExample(t *suite.T) {
    t.Equal(5, 5)
}

Related issues

Fixes #934, #187.

@maroux maroux force-pushed the suite_copy branch 2 times, most recently from 8f0fcaa to 3a63e1d Compare August 4, 2021 05:12
Define new interface `Copy` to create copies of suite object for parallel subtests
@maroux maroux changed the title Support for parallel sub-tests Add support for parallel sub-tests and remove suite.Suite pseudo-interitance Nov 30, 2021
@ivanpk
Copy link

ivanpk commented Dec 10, 2021

Thanks for this,

Additionally, since there's only one T object per suite instance, this means during parallel
tests, this object gets overridden by subsequent tests's F method, which leads to test output being associated with the wrong test,
and potentially other problems because testing.T is NOT concurrent-safe.

is the main fix I'm looking forward to.

@nightah
Copy link

nightah commented May 12, 2022

I can see that this is tagged with the v1.7.0 milestone, is this likely to be merged before v2 at this stage?

Really keen on being able to run parallel tests within suites.

@maroux
Copy link
Author

maroux commented May 13, 2022

is this likely to be merged before v2 at this stage?

No. Given that this is a breaking change.

@LorisFriedel
Copy link

Great PR! Looking forward to it :)

@FedorLap2006
Copy link

FedorLap2006 commented Aug 26, 2022

Any progress? 👀

mloo3 added a commit to mloo3/testify that referenced this pull request Oct 3, 2022
mloo3 added a commit to mloo3/testify that referenced this pull request Oct 3, 2022
Add support for parallel sub-tests and remove suite.Suite pseudo-interitance stretchr#1109

See merge request prismacloud/pcn/testify!1
@mstrYoda
Copy link

mstrYoda commented Mar 9, 2023

Any progress on this?

@dolmen
Copy link
Collaborator

dolmen commented Jul 25, 2023

v2 will not happen.

Please publish this as a separate module that YOU will maintain.

@dolmen dolmen added pkg-suite Change related to package testify/suite must-rebase labels Jul 25, 2023
@pmenglund
Copy link

@dolmen have there been any public post about v2 not happening?

@dolmen
Copy link
Collaborator

dolmen commented Jul 25, 2023

@pmenglund Have there been any public post about v2 happening?

The fact is that maintaining v1 is a huge effort. Just look at the count of open issues and PRs. Nobody is paid to work on this project. I'm currently doing a massive triage, but I keep seeing new issues and PRs coming almost every day. And maintainers ressources are scarce. The API surface continues to grow with PRs, but most contributors are gone once their feature is merged.

v2 will not magically make v1 disappear. v2 will just make maintenance harder because both v1 and v2 will have to be maintained by the same people.

Packages assert/require/mock v1 are good enough for most people. Also I know that most projects which use v1 will never move away from v1: who wants to take the risk of breaking their test suite for no benefits?

I'm also seeing right now the consequences of PRs having been merged without the care needed for such a popular project (for example, public APIs introduced in recent patch releases. Also API surface being increase with badly designed APIs).

So if someone wants to make a v2 with APIs cleanup, just use a separate module path.

@pmenglund
Copy link

@dolmen there was no critique whatsoever in my question, I know what it takes to run an open source project in your spare time. I was just curious if I had missed some announcement since the README still says "We are working on testify v2 ..."

@dolmen
Copy link
Collaborator

dolmen commented Jul 26, 2023

@pmenglund Thanks for the pointer. I will propose something to change the paragraph.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change concurrency must-rebase pkg-suite Change related to package testify/suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TearDownTest is being executed while parallel sub tests are still running
8 participants