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

Some tests in pkg/analyzer_experimental/test/ are too big #11230

Closed
peter-ahe-google opened this issue Jun 12, 2013 · 14 comments
Closed

Some tests in pkg/analyzer_experimental/test/ are too big #11230

peter-ahe-google opened this issue Jun 12, 2013 · 14 comments
Assignees
Labels
area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). closed-obsolete Closed as the reported issue is no longer relevant

Comments

@peter-ahe-google
Copy link
Contributor

Unlike junit, test.py/test.dart is designed to run small standalone tests: one file per test.

When porting tests to Dart, you must ensure that the result is small standalone tests that complete in less than a second.

I have observed pkg/analyzer_experimental/test/generated/ast_test timing out.

@bwilkerson
Copy link
Member

Added this to the M6 milestone.
Removed Priority-Unassigned label.
Added Priority-Medium label.

@bwilkerson
Copy link
Member

I'm not sure how best to deal with this. We could probably change the translator to generate each test into it's own file, but I'm not sure that's the right answer.

Are you suggesting that all of our tests should be like the language and co19 tests? Those are fine integration tests (although there are improvements I would like to see made to them), but I value being able to have unit tests as well. It really doesn't seem reasonable to not be able to run unit tests.

Perhaps we're just not running the tests the right way. Given a file that uses the unittest package to define multiple unit tests, how should we be running them?

@peter-ahe-google
Copy link
Contributor Author

Brian, it sounds like the right answer to me to generate many small files.

I strongly recommend against trying to dynamically split up tests. It doesn't scale, for example, dart2js will still take a long time compiling the test.

@bwilkerson
Copy link
Member

Removed this from the M6 milestone.
Added this to the M7 milestone.

@jwren
Copy link
Member

jwren commented Sep 19, 2013

M7 -> M8


Removed this from the M7 milestone.
Added this to the M8 milestone.

@bwilkerson
Copy link
Member

Removed this from the M8 milestone.
Added this to the M9 milestone.

@clayberg
Copy link

Removed this from the M9 milestone.
Added this to the 1.1 milestone.

@clayberg
Copy link

Removed this from the 1.1 milestone.
Added this to the 1.2 milestone.

@bwilkerson
Copy link
Member

Removed this from the 1.2 milestone.
Added this to the Later milestone.

@bwilkerson
Copy link
Member

I still don't know what to do about this. I really don't think it's reasonable to put every unit test in a separate file, and it doesn't look like other packages have their tests organized that way. Is there an approved way to structure unit tests written in Dart?


Set owner to @ricowind.
Removed Area-Analyzer label.
Added Area-Test label.

@peter-ahe-google
Copy link
Contributor Author

Perhaps you should look to dart2js for inspiration, rather than other packages. The analyzer and dart2js has a lot in common: large code base and complex tests.

In dart2js, we try to keep unit tests small. See tests/compiler/dart2js.

But I think you may have solved this by improving performance of the analyzer. I haven't seen the test time out in a while, and it seems to complete within reasonable time.

However, please keep in mind that our test framework is designed to run small tests, not big test suites like JUnit, so you generally don't want to have hundreds of tests per file. Also, keep in mind that when you look at what other packages do, you'll generally find a lot of examples of things that don't fit well with test.dart.

@kasperl
Copy link

kasperl commented Jul 10, 2014

Removed this from the Later milestone.
Added Oldschool-Milestone-Later label.

@kasperl
Copy link

kasperl commented Aug 4, 2014

Removed Oldschool-Milestone-Later label.

@ricowind
Copy link
Contributor

ricowind commented Jun 2, 2015

Added AssumedStale label.

@peter-ahe-google peter-ahe-google added Type-Defect area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). closed-obsolete Closed as the reported issue is no longer relevant labels Jun 2, 2015
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). closed-obsolete Closed as the reported issue is no longer relevant
Projects
None yet
Development

No branches or pull requests

6 participants