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

Difficult to call t.Parallel with td.T #149

Closed
abhinav opened this issue Jul 10, 2021 · 3 comments · Fixed by #150
Closed

Difficult to call t.Parallel with td.T #149

abhinav opened this issue Jul 10, 2021 · 3 comments · Fixed by #150

Comments

@abhinav
Copy link
Contributor

abhinav commented Jul 10, 2021

testdeep encourages wrapping the *testing.T into a *td.T. This
unfortunately makes it difficult to call t.Parallel() on the
*testing.T once hidden behind the *td.T.

func TestFoo(tt *testing.T) {
    t := td.NewT(tt)
    
    t.Run("foo", func(t *td.T) {
        // t.Parallel() <<< errors because Parallel() does not exist
        //                  on the testing.TB interface.
        
        t.Cmp(...)
    })
    
    t.Run("bar", func(t *td.T) {
        // t.Parallel()
        
        t.Cmp(...)
    })
}

It's possible to call t.Parallel() on the *testing.T with some
upcasting on the wrapped testing.TB.

func Parallel(t *td.T) {
	tp, ok := t.TB.(interface{ Parallel() })
	if ok {
		tp.Parallel()
	}
}

If you're open to addressing this within testdeep, there are two
straightforward options:

  • Add a top-level func Parallel(*td.T) that does the same as above.
    Usage would then be,

    t.Run("foo", func(t *td.T) {
        td.Parallel(t)
        
        t.Cmp(...)
    })
  • Add a Parallel() method on td.T. Usage:

    t.Run("foo", func(t *td.T) {
        t.Parallel()
        
        t.Cmp(...)
    })

The latter is closest to what people already do with *testing.T so it
might be preferable.


Separately, either choice opens room for a setting in testdeep to
always call t.Parallel() for subtests invoked with a specific
*td.T.Run. For example,

func TestFoo(tt *testing.T) {
    t := td.NewT(tt).AlwaysParallel(true)
    
    t.Run("foo", func(t *td.T) {
        // already parallel
        t.Cmp(...)
    })
    
    t.Run("bar", func(t *td.T) {
        // already parallel
        t.Cmp(...)
    })
}

But that can be a separate discussion.

@maxatome
Copy link
Owner

Hi, good proposals. Let me review your PR and think about it after some rest and I come back to you. Thanks!

@maxatome
Copy link
Owner

maxatome commented Jul 12, 2021

#150 just reviewed.

After some thought, I am not sure AlwaysParallel() method is a good idea. I understand the concern, but won't this setting be unusual for the readers? As usually each subtest has to be explicitly set as parallelizable, it could be non-obvious to understand why a test runs in parallel while no Parallel() call is around.

@abhinav
Copy link
Contributor Author

abhinav commented Jul 12, 2021

#150 just reviewed.

After some thought, I am not sure AlwaysParallel() method is a good idea. I understand the concern, but won't this setting be unusual for the readers? As usually each subtest has to be explicitly set as parallelizable, it could be non-obvious to understand why a test runs in parallel while no Parallel() call is around.

Reasonable. That was just something that could be possible once Parallel was supported but obviously an optional feature. If you feel that the confusion it would cause is more than its value, we can omit it.

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 a pull request may close this issue.

2 participants