chore: properly isolate module tests#16712
Conversation
angularFiles.js
Outdated
| 'karmaModulesBase': [ | ||
| 'build/angular.js', | ||
| '@angularSrcModules', | ||
| 'src/ngMock/*.js', |
There was a problem hiding this comment.
Does this have to be here? Can't it shadow problems as before (e.g. modules depending on stuff that is defined in ngMock's source code)?
There was a problem hiding this comment.
the angular-mocks file defines the module fn that integrates with jasmine, so I think it unfortunately must always be included:
angular.js/src/ngMock/angular-mocks.js
Line 3085 in bb5a7e3
There was a problem hiding this comment.
But we get the same integration with build/angular-mocks.js, right? (Without the problem of making things defined in the ngMocks closure available to other modules.)
I would expect build/angular-mocks.js to be included in all module tests, except for ngMocks specs, which should indeed use src/ngMock/*.js. Do I miss something?
There was a problem hiding this comment.
Using build/angular-mocks.js works great, good suggestion!
In the beginning, it was a big hassle to separate all these files, it's possible that at some point the ngMock src files had to be included.
|
|
||
| 'karmaModules-ngMessages': [ | ||
| '@karmaModulesBase', | ||
| 'build/angular-animate.js', |
There was a problem hiding this comment.
Some tests in ngMessages are dependent on ngAnimate, e.g.
angular.js/test/ngMessages/messagesSpec.js
Line 465 in bb5a7e3
There was a problem hiding this comment.
Do we still need ngAnimate if we use ngAnimateMock?
There was a problem hiding this comment.
Theoretically yes, but I'm more comfortable with testing the actual animations instead of just the base provided by ng.
angularFiles.js
Outdated
|
|
||
| 'karmaModules-ngMock': [ | ||
| '@karmaModulesBase', | ||
| 'build/angular-resource.js', |
There was a problem hiding this comment.
There's a test that uses ngResource:
angular.js/test/ngMock/angular-mocksSpec.js
Line 1114 in bb5a7e3
I think we could use a custom module here instead.
607e3d1 to
86d916a
Compare
711f06a to
2f1f6d9
Compare
2f1f6d9 to
85747ae
Compare
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behavior? (You can also link to an open issue here)
Previously, our module tests weren't running in isolation. Theroetically, we wanted to test if the angular.js build file works well with the individual module source files (Note that we don't use the module build files because we sometimes test non-public apis).
However, we bundled all module files together with the build file, and this messed up the results as some modules assign angular helpers like isDefined in the current scope, which means all following modules have access to them too, which essentially defeats the purpose of these tests. See #16702.
What is the new behavior (if this is a feature change)?
Does this PR introduce a breaking change?
Please check if the PR fulfills these requirements
Other information: