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

Change @isTestGroup to @isTest on testGoldens #140

Merged
merged 1 commit into from
Jan 11, 2022

Conversation

DanTup
Copy link
Contributor

@DanTup DanTup commented Jan 11, 2022

The testGoldens function currently has an @isTestGroup annotation, but the function just represents a test, not a group. It looks like it used to contain a group, but that was removed in favour of tags in f1d486c. This annotation appears to have been overlooked.

With this set to group, the VS Code extension fails to merge the results from running the tests (where it gets a test with this name) and the analyzer discovery results (where it gets a group based on the annotation). The error has been fixed in Dart-Code/Dart-Code#3776, however even with that fix, the tree will now show both a test and a group with this name (since groups and tests are never merged together).

Changing the annotation to @isTest fixes this, and everything works as expected.

Related PRs/commits:

This method represents a test, not a group of tests. This annotation is used by editors like VS Code to locate tests/groups and merges them with the test results in the tree. Using `isTestGroup` would cause the analysis-based test discovery to add a group with this name, whereas the test results would add a test with this name. Both should be the same.
@DanTup
Copy link
Contributor Author

DanTup commented Jan 11, 2022

I believe the bot failures are unrelated to this change. Some seem to be that dartfmt doesn't exist anymore (should be dart format), and some are errors about duplicate imports. I presume master would fail on these for the same reasons.

@coreysprague
Copy link
Contributor

thanks @DanTup

I just got back from vacation and am sifting through PRs. You are correct about the dart format. I'll get the CI up to snuff and get this merged. thanks for the contribution!

Copy link
Contributor

@coreysprague coreysprague left a comment

Choose a reason for hiding this comment

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

👍

@coreysprague coreysprague merged commit fbb1564 into eBay:master Jan 11, 2022
@coreysprague
Copy link
Contributor

fix published in 0.13.0

@DanTup
Copy link
Contributor Author

DanTup commented Jan 11, 2022

Thanks!

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.

2 participants