Fix EZP-21671: add tests for Promise part of the CAPI

[wiseman17]

JIRA issue: https://jira.ez.no/browse/EZP-21671

Done.

[dpobel]

this is more a workaround than a real solution and actually this happens because Q is loaded with a relative path to node_modules which is considered as a bad practice. So instead of this, you should try to get rid of the relative path to load Q. (normally npm packages are loaded automatically by requirejs/r.js but I guess the baseUrl being set to src breaks this somehow)

[wiseman17]

Well, I didn't find a way to load "q" properly and without much hassle yet.

2 alternatives to achieve "best practice":

1. copy actual q into "src/" before the build. I realize, that it's not nice, but still :)
2. change sourceUrl to the / and prefix each module name with "src/"

I've managed to replace copying hack by usage of the map parameter in requirejs config.
Damien, what do you think about this (see 2 last commits) - more elegant but still "not best practice" solution?

[dpobel]

Another solution might be to install Q in src (with a grunt task) before building or running the unit tests and in test/instrument/src before running the test coverage so that we don't have to reference the node_modules anywhere.

[wiseman17]

Do you mean actual installing via npm?
I guess it can be troublesome: http://stackoverflow.com/questions/14469515/how-to-npm-install-to-a-specified-directory

[dpobel]

I was more thinking of putting a package.json file in those folders with the Q dependency. Maybe @jakobwesthoff you can give your pov or an alternative as none of those solutions seems really perfect.

[jakobwesthoff]

I would split this up into two tests. As ensuring the services are correctly called is a different requirement, then them being singletons :)

[jakobwesthoff]

Regarding the problem of loading Q

Loading the Q library is indeed a problem. First of all it needs to be decided if Q should be bundled with the CAPI or not. I tend to not bundle it, but simply require it as a dependency if the PromiseCAPI is used. Furthermore I tend to distribute/create two versions of the CAPI. One including the Promise wrappers and therefore having a dependency on Q, one without the Promise wrappers, without any external dependency.

This would solve the problem regarding the distribution of Q. The second problem is the need for Q during the execution of tests.

Unfortunately there is no really nice way of solving this problem. The currently utilized solution is quite acceptable in my opinion. However I would use paths instead of map. As map is used to allow different modules inside the application utilize different versions of a dependency, while path simply tells requirejs, where it has to look for a certain module. In our case each time q is required the same dependency is loaded therefore a path entry should suffice:

paths: {
  "q": "../node_modules/q/q"
}

The corresponding documentation can be found here.

Another solution the grunt-contrib-symlink plugin to symlink Q from the node_modules folder to the corresponding src folder, before initiating the require.js build. However simply defining the paths correctly for require.js to know where its dependencies are seems to be a cleaner solution.

[dpobel]

big +1 on having separate builds of the promise and non promise version. For Q not being included, I tend to agree as well even if I don't have a strong opinion on this.

[jakobwesthoff]

The reason for not including Q (even int the promise build) was to give the user of the library the highest possible flexibility regarding choice of the used package management (requirejs, manual download, commonjs, ...). As we are able to provide this flexibility to the user by utilizing requirejs, I don't see a real reason for taking part of this away by bundling Q statically. Nevertheless it is not a must have :)

[wiseman17]

Jakob, thanks for additional remarks. I'll fix them right away.
I guess that answers our most painstaking questions.

@dpobel, ok with you if I'll fix the test issues here, merge them, and then implement separate builds in a new branch?

[dpobel]

of course in a separate branch

[wiseman17]

Fixed.

[dpobel]

seems OK to me. Just make sure q.stopUnhandledRejectionTracking(); things does not end up in the master branch while merging.