Skip to content

Conversation

@matanlurey
Copy link
Contributor

Closes #160.

Also added test cases, which in turn found a bug. Whad'ya know @jakemac53 🐑

Closes #161.

Copy link
Member

@kevmoo kevmoo left a comment

Choose a reason for hiding this comment

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

LGTM – waiting on @jakemac53

/// because it relies on knowing the _absolute_ path (i.e. after resolved
/// `export` directives). You should ideally only use `fromUrl` when you know
/// the full path (likely you own/control the package) or it is in a stable
/// package like in the `dart:` SDK.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe "stable package or the Dart SDK via dart: URLs.

void main() {
LibraryElement library;

setUpAll(() async {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious, I notice you use this setUpAll a lot but I don't understand what the value is over just regular setup in the top of main?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

setUpAll is only invoked once, not once per test.

The body of this function (doing analysis) is expensive, and doesn't change between tests, so this makes the test take ~20s instead of minutes.

Copy link
Contributor

Choose a reason for hiding this comment

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

main is only invoked once as well :).

I talked with @kevmoo and I guess you get better errors if the setup fails at least so there is that advantage. Also if all tests end up skipped then it wouldn't execute this code.

Copy link
Contributor Author

@matanlurey matanlurey Jun 20, 2017

Choose a reason for hiding this comment

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

Yeah, also it makes the use of async a little cleaner.

@matanlurey matanlurey merged commit c3ea180 into dart-lang:master Jun 20, 2017
@matanlurey matanlurey deleted the find-type branch June 20, 2017 15:23
mosuem pushed a commit to dart-lang/build that referenced this pull request Dec 10, 2024
mosuem pushed a commit to dart-lang/build that referenced this pull request Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants