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

recreate the context variable for parallel test #8748

Merged
merged 1 commit into from
Mar 13, 2023
Merged

Conversation

mangalaman93
Copy link
Member

@mangalaman93 mangalaman93 commented Mar 11, 2023

Also, use require.NoError instead of require.Nil for ensuring that error is nil. This prints better when err is not nil.

@dgraph-bot dgraph-bot added area/enterprise Related to proprietary features area/enterprise/acl Related to Access Control Lists area/testing Testing related issues go Pull requests that update Go code labels Mar 11, 2023
Also, use require.NoError instead of require.Nil for ensuring
that error is nil. This prints better when err is not nil.
@all-seeing-code
Copy link
Contributor

all-seeing-code commented Mar 13, 2023

@mangalaman93 The PR is fine, just wondering how do you know that this should fix the flakyness? PR looks no-op to me.

Also, use require.NoError instead of require.Nil for ensuring that error is nil. This prints better when err is not nil.

Before and after would help understand the actual difference.

@@ -1892,7 +1892,7 @@ func TestNewACLPredicates(t *testing.T) {
tc := tc // capture range variable
t.Run(tc.description, func(t *testing.T) {
t.Parallel()
ctx, cancel = context.WithTimeout(context.Background(), 100*time.Second)
ctx, cancel := context.WithTimeout(context.Background(), 100*time.Second)
Copy link
Member Author

Choose a reason for hiding this comment

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

@anurags92 this is the change that could fix the flakiness. The rest of the change is so that we see the errors better in the logs.

@mangalaman93 mangalaman93 merged commit a43743c into main Mar 13, 2023
@mangalaman93 mangalaman93 deleted the aman/aclfix branch March 13, 2023 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/enterprise/acl Related to Access Control Lists area/enterprise Related to proprietary features area/testing Testing related issues go Pull requests that update Go code
Development

Successfully merging this pull request may close these issues.

4 participants