Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Running angular-mocks tests concurrently #13971

Closed
karlhorky opened this issue Feb 8, 2016 · 18 comments
Closed

Running angular-mocks tests concurrently #13971

karlhorky opened this issue Feb 8, 2016 · 18 comments

Comments

@karlhorky
Copy link

I've been playing around with testing Angular directives in the AVA concurrent test runner and I've run into issues with getting the tests running concurrently. It appears angular-mocks rewrites variables in the afterEach() method, causing property reference errors:

Partial test setup

import test from 'ava';

// ...

global.window.beforeEach = test.beforeEach;
global.window.afterEach = test.afterEach;
require('angular-mocks');

// ...

test('a', t => t.true(true));
test('b', t => t.true(true));

Command line output (annotated with comment lines)

$ ava --verbose

# beforeEach for "a" called
# beforeEach for "b" called
  ✔ a
  ✔ b
# afterEach for "a" called  <-- currentSpec set to null here
  ✖ afterEach for "b" failed with "Cannot read property '$injector' of null"

  1 test failed

  1. afterEach for "b"
  TypeError: Cannot read property '$injector' of null
    Test.fn (node_modules/angular-mocks/angular-mocks.js:2593:31)

Relevant code from angular-mocks

  (window.beforeEach || window.setup)(function() {
    // ...
    currentSpec = this;
  });

  (window.afterEach || window.teardown)(function() {
    var injector = currentSpec.$injector;

    // ...

    currentSpec.$injector = null;
    currentSpec.$modules = null;
    currentSpec.$providerInjector = null;
    currentSpec = null;

    // ...

  });

Rewriting the tests to execute serially works around this error (but loses the nice property of concurrency):

test.serial('a', t => t.true(true));
test.serial('b', t => t.true(true));

A full example including failing and working cases here:
https://gist.github.com/karlhorky/6e68101d8167be27e027

Is concurrent tests something that the angular team is interested in supporting? If not, are there any other workarounds for getting angular-mocks running concurrently?

angular.js v. 1.5.0
angular-mocks v. 1.5.0

@petebacondarwin
Copy link
Contributor

We have no plans to support concurrent testing and so AVA is probably not something that is going to work with AngularJS.
If you wanted to make the tests more concurrent, you could consider sharding the test base and running multiple completely separate karma instances.

@karlhorky
Copy link
Author

Hm, ok that's too bad. Concurrent tests can really speed up test runs.

I guess the reason for this is that effort on new things like this is being put into Angular 2 now?

@karlhorky
Copy link
Author

Thanks for the suggestion for karma, but I'm trying a minimal testing setup (trying to avoid too many tools). AVA is both the test framework and the runner.

I've found a workaround for the issue to still maintain concurrency - running the window.afterEach || window.teardown only once as a test.after call (instead of test.afterEach):

- global.window.afterEach = test.afterEach;
+ global.window.afterEach = test.after;

@karlhorky
Copy link
Author

@kentcdodds long shot but I've seen some of your projects have included AVA lately and I know you know Angular, do you have any input on this?

@kentcdodds
Copy link
Member

I think if you try to retrofit Angular tests to work with ava, you're going to have a hard time I'm afraid. But I've never tried.

@kentcdodds
Copy link
Member

Note, main reason for this is ava does not support running tests in the browser. And Angular 1 needs the browser APIs to be tested reliably.

You could, however, write many of your services in vanilla JS, and test those in isolation.....

@karlhorky
Copy link
Author

I'm running Angular in jsdom to emulate the browser environment: https://gist.github.com/karlhorky/6e68101d8167be27e027

The issue is mainly just angular-mocks and its mutation of shared state (currentSpec) across multiple beforeEach and afterEach calls. But I guess it's not something that anyone has taken a shot at fixing before.

@gkalpak
Copy link
Member

gkalpak commented Feb 8, 2016

I'm not familiar with AVA, but is the problem really that angukar-mocks mutates shared state or the fact that this state (e.g. the injector is shared among specs ?

@karlhorky
Copy link
Author

I'm still learning how AVA works as well, but in my first few directive unit tests, the shared state itself doesn't appear to cause problems in the concurrent tests. I'm still able to initialize the module and inject services and providers like usual. The tests seem to run just like mocha.

The issue appears to be the following sequence:

window.afterEach = test.afterEach;

beforeEach 'test 1' 
beforeEach 'test 2'
✔ test 1
✔ test 2
afterEach 'test 1'  <-- currentSpec set to null here
afterEach 'test 2'  <-- currentSpec.$injector referenced here (error)

When I set the window.afterEach function in the angular-mocks scope to be run after all tests (test.after, similar to Jasmine's afterAll), the dereferencing of currentSpec doesn't happen until the very end, and no property reference errors occur.

window.afterEach = test.after;

beforeEach 'test 1' 
beforeEach 'test 2'
✔ test 1
✔ test 2
after() <-- currentSpec set to null here

@karlhorky
Copy link
Author

@sindresorhus @jamestalmage any feedback on running angular-mocks with AVA? The relevant part of angular-mocks is here:

var currentSpec = null,
annotatedFunctions = [],
isSpecRunning = function() {
return !!currentSpec;
};
angular.mock.$$annotate = angular.injector.$$annotate;
angular.injector.$$annotate = function(fn) {
if (typeof fn === 'function' && !fn.$inject) {
annotatedFunctions.push(fn);
}
return angular.mock.$$annotate.apply(this, arguments);
};
(window.beforeEach || window.setup)(function() {
annotatedFunctions = [];
currentSpec = this;
});
(window.afterEach || window.teardown)(function() {
var injector = currentSpec.$injector;
annotatedFunctions.forEach(function(fn) {
delete fn.$inject;
});
angular.forEach(currentSpec.$modules, function(module) {
if (module && module.$$hashKey) {
module.$$hashKey = undefined;
}
});
currentSpec.$injector = null;
currentSpec.$modules = null;
currentSpec.$providerInjector = null;
currentSpec = null;
if (injector) {
injector.get('$rootElement').off();
injector.get('$rootScope').$destroy();
}
// clean up jquery's fragment cache
angular.forEach(angular.element.fragments, function(val, key) {
delete angular.element.fragments[key];
});
MockXhr.$$lastInstance = null;
angular.forEach(angular.callbacks, function(val, key) {
delete angular.callbacks[key];
});
angular.callbacks.counter = 0;
});

My setup: https://gist.github.com/karlhorky/6e68101d8167be27e027

@gkalpak
Copy link
Member

gkalpak commented Feb 8, 2016

If all concurrent tests share the same $injector (again, I don't know if that's the case with AVA), you will not always run into problem. But things will start to break in unexpected and hard-to-debug ways, when one test affects this shared state in a way that another test does not anticipate.

Possible issues (off the top of my head):

  • One test needs to mock a service that another test depends on.
    (Remeber that services are singletons inside an $injector instance.)
  • Have both tests call the same spied service method and trying to assert it has been called a specific number of times with specific items. Each test, won't know about the calls made by the other test.
  • Calling $rootScope.$digest() or $timeout.flush() or $httpBackend.flush() etc from one test, will affect the other.

@kentcdodds
Copy link
Member

Yeah, it's these global state things that make testing complex that ava is striving to solve by isolating tests from one another. You'll probably be able to make it work with enough work, but personally, I don't know whether it's worth the effort to get ava and angular to play nice.

@jamestalmage
Copy link
Contributor

My advice would be to write the majority of your services as ES2015/CommonJS modules with no Angular related dependencies, and move as much of your code in there as possible.

For modules that must use Angular APIs and are affected by Angulars global state, use test.serial and test the old fashioned way.

This advice could be adapted to apply to any app built on any framework.

@karlhorky
Copy link
Author

@gkalpak ah ok, thanks for the explanation. I think I'm starting to understand what the difficulties could be here.

I wonder if there would be a way to isolate tests to avoid side effects like these. Maybe I'd need to move the tests needing isolation into separate files (AVA provides an isolated environment for each test file). I would just do the injection and mocking manually in an isolated way, but I don't think I quite understand angular-mocks well enough for that yet and also it kind of kills productivity when you just want to write some tests.

Maybe @kentcdodds is right and it's not worth the effort. ...On the other hand, concurrent tests written in ES2015+ are pretty appealing...

@karlhorky
Copy link
Author

@jamestalmage hm, that's an interesting approach, thanks.

I'll have to think about how I could rearchitect my module structure for something like that. Currently all of my ES2015 modules include code attaching to angular modules and injecting dependencies using the injector.

@jamestalmage
Copy link
Contributor

If you use test.serial(...) everywhere all the test files still will run concurrently, only the tests within a given file will run serially to each other. Since Angular provides ways of making most of your async tests synchronous ($timeout.flush()) - there's really very little benefit to running tests concurrently within a given test file amyways. JavaScript is still single threaded, so "concurrent" tests in the same file that are synchronous, are pretty much equivalent to serial tests.

@sindresorhus
Copy link

Sounds like a good candidate for a recipe.

@petebacondarwin
Copy link
Contributor

Closing as we are not going to change Angular to fix this issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants