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

suite: fix subtest names (fix #1501) #1504

Merged
merged 2 commits into from Nov 9, 2023
Merged

Conversation

dolmen
Copy link
Collaborator

@dolmen dolmen commented Nov 2, 2023

Summary

We are doing dirty things with testing.InternalTest. It looks like test names should have the same prefix as the parent test, like if they were true subtests (testing.T.Run). So until we drop our usage of testing.InternamTest, let's rename the tests.

Changes

  • Rename the tests when calling testing.RunTests to use the name of the parent test as prefix.
  • Also added a few checks of the testing.RunTests result.

Motivation

CI failure (#1501).

Related issues

The subtest of TestSubtestPanic is expected to fail, but that must not
make the testuite of package 'suite' to fail. So call Skip to make 'go
test' to ignore that expected failure.
We are doing dirty things with testing.InternalTest. It looks like test
names should have the same prefix as the parent test, like if they were
true subtests (testing.T.Run).

So until we drop our usage of testing.InternamTest, let's rename the
tests.

Also added a few checks of the testing.RunTests result.
@dolmen
Copy link
Collaborator Author

dolmen commented Nov 2, 2023

Following my announcement on Slack, I'm pinging all other co-maintainers.

@dolmen dolmen changed the title suite: fix subtest names suite: fix subtest names (fix #1501) Nov 2, 2023
@nelsam nelsam removed their request for review November 2, 2023 15:24
@hendrywiranto
Copy link
Contributor

hendrywiranto commented Nov 6, 2023

looks like the other maintainers are very slow to respond, I can help by opening the PR so you can approve and merge it instead (if you don't mind ofc)
please let me know if you need any help, and thanks for maintaining this repo @dolmen

@boyan-soubachov boyan-soubachov merged commit db8608e into master Nov 9, 2023
14 checks passed
@dolmen dolmen deleted the suite-fix-subtest-names branch November 9, 2023 15:32
@dolmen
Copy link
Collaborator Author

dolmen commented Nov 9, 2023

Thanks @boyan-soubachov for the merge !

Thanks @hendrywiranto for giving a hand. Right now any review you could do on the long list of pending PRs would be useful (use the "Review" button in GitHub).

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.

None yet

4 participants