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

Declare addTearDownLibrary. #2154

Closed
wants to merge 3 commits into from
Closed

Declare addTearDownLibrary. #2154

wants to merge 3 commits into from

Conversation

polina-c
Copy link

@polina-c polina-c commented Dec 11, 2023

Prerequisite for flutter/flutter#138057.
TODO: add test coverage.

@polina-c polina-c requested a review from natebosch December 11, 2023 01:25
@natebosch
Copy link
Member

Why do we need this over setUpAll and addTearDown?

@polina-c
Copy link
Author

polina-c commented Dec 11, 2023

Why do we need this over setUpAll and addTearDown?

This method will tear down all tests in the library, not all tests in the group. I do not see other way to traverse to root group. Am i missing something?

@natebosch
Copy link
Member

Why do we need this over setUpAll and addTearDown?

Because we need it to tear down all tests in the library, not all tests in the group. I do not see other way to traverse to root group. Am i missing something?

I thought the plan was to intercept the call to group and inject the setUpAll call?

If we want to avoid that because it's too brittle we can try something like this. I'd go with the name addTearDownSuite. @jakemac53 any concerns about such an API?

@natebosch
Copy link
Member

natebosch commented Dec 11, 2023

Ah this isn't user facing either. Should be pretty safe then. Let's rename to "suite" and add a changelog entry.

Nope, we will need to change flutter_test/lib/src/test_compat.dart in order to use this API.
There are two ways: interception for group and others (internal) and added api (user facing).

@polina-c polina-c closed this Dec 12, 2023
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