update to the test package #84
Conversation
| /// Custom implementations of the functions from `package:test`. These ensure | ||
| /// that the body of all test function are run in the dirty checking zone. | ||
| test(String description, body()) => original_test.test( | ||
| description, () => dirtyCheckZone().bindCallback(body)()); |
There was a problem hiding this comment.
cool that you figured it out!
@nex3 - is there anything that can be changed in the test package to make it easier to work with code that uses zones without having to add workarounds like this one?
There was a problem hiding this comment.
I'm not sure what's bad about this solution. If you want a test body to be run in a zone, run it in the zone. It seems like the most straightforward way to express what you mean.
There was a problem hiding this comment.
The only issue is that previously we could define a single top level function which runs all tests, and just run that one function in the zone (all the test bodies would also be ran in the same zone). This is no longer the case in the test runner.
There was a problem hiding this comment.
What do you mean by "a function that runs all tests"? Is this something you were defining on Configuration?
There was a problem hiding this comment.
I just mean a top level function which contained all the invocations of test, see https://github.com/dart-lang/observe/pull/84/files#diff-d0b4c4bde478579a03ae3898a56bb2edL13.
There was a problem hiding this comment.
I suppose we could bind test callbacks to the zone they're declared in, but if the zone is stateful then you're setting yourself up for hidden state dependencies between tests.
Another possibility would be to provide a surround() function that would work like a combination of setUp() and tearDown(). For example:
void main() {
surround((runTest) {
dirtyCheckZone().bindCallback(runTest)();
});
// ...
}I'm not sure this is feasible—there are a lot of tricky corner cases about how it would interact with tearDown(), errors, and asynchrony that I'd probably have to implement it to fully understand—but if it is, would it help this case?
There was a problem hiding this comment.
One other note: it's important to be sure that the test package's own zones are still in the body's zone hierarchy. Does your dirtyCheckZone() function do that?
There was a problem hiding this comment.
I suppose we could bind test callbacks to the zone they're declared in, but if the zone is stateful then you're setting yourself up for hidden state dependencies between tests.
Good point, although I don't think it would be any worse than the solution implemented here in that regard.
Another possibility would be to provide a surround() function that would work like a combination of setUp() and tearDown()
This sounds like it would help, but I don't know if its worth investing time in or not.
There was a problem hiding this comment.
This sounds like it would help, but I don't know if its worth investing time in or not.
May as well file an issue: dart-lang/test#337
|
lgtm |
No description provided.