Fix for failing 'gulp test-coverage' command

[Fawke]

Type of change

• Other

Description of change

Gulp command gulp test-coverage has been failing with the following error:

Suspected cause of this failure is the commit related to this PR: #5092

The error is originating from the utils.js file. The currently generated source map is not able to accurately point out the exact location of the error for me to fix it!

I've currently removed this file, src/utils.js from our code-coverage checks to make the command work again since it's critical to check test coverage of new PRs (especially bid adapters) as it's an important part of code review process.

[Fawke]

@snapwich Would you mind having a look? I tried to fix the command, but could not do so without commenting out the utils.js file.

[snapwich]

So I've fixed this once before, the issue is with the instanbul-instrument-loader, you can see my comment here: webpack-contrib/istanbul-instrumenter-loader#83

This really is an issue with that plugin, it doesn't like circular dependencies even though ES6 allows circular dependencies and works correctly in our situation (as you can see the tests pass just fine).

So if we want to continue using that plugin I guess we need to remove the circular dependency. I'm of pretty strong opinion that utils.js should not be importing any other internal modules because that then makes those modules unable to import utils.js without creating a circular dependency. Right now config.js depends on that url parsing as well as the mergeDeep that was added in #4950, both of those should be fine and config should be allowed to depend on utils. utils.js depends on config to know if debug mode is turned on... Either we can move some dependencies around, combine the two modules (config and utils), or some other solution. Maybe the solution is to get rid of this broken plugin (instanbul-instrument-loader) that is no longer supported?

[Fawke]

@snapwich Thanks for pointing out that circular dependency issue. It would've taken ages for me out the cause of the error on my own.

I've now replaced karma-instrumenter-loader with a different module. It's uses a new version of the Istanbul api and I no longer see the error.

Although, please note that in order to use this plugin, you have to upgrade your Node version to 12. I was getting a weird error on Node version 10.

You can check the command by running gulp test-coverage followed by gulp view-coverage

[robertrmartinez]

Thanks for fixing this.

Are there any repercussions to requiring Node > 12?

We state in the README Note: You need to have NodeJS 8.9.x or greater installed.

So we will need to update this, right?

Or is this really only going to affect people who want to run gulp test-coverage?

[robertrmartinez]

We have run into issues on-boarding new devs who get their new laptops and have the latest and greatest npm and node versions installed, and the only way to get their prebid environment working is to have them use older npm and node versions.

I am not super keen on versioning and all that so obviously I defer to the more practiced users! But just wanted to ask a couple questions to be sure.

[jsnellbaker]

There are some other packages that need to be updated to work with the newer Node versions. I recall this issue noted some of the packages that need to addressed #3878 but there may be others as well.

[mike-chowla]

When I try to use node v12, npm install fails due [email protected] being incompatible with Node v12. Fibers 3.1.1 by [email protected] which is really old version that package. The current version of it is 6.1.0

[Fawke]

I've upgraded the package, wdio-mocha-framework to the latest version, 6.1.0. Now, npm install should not throw any error with Node v12.

[Fawke]

I just realised the command gulp e2e-test --host=test.localhost is failing because webdriverio v4.13.2 is not compatible with wdio/mocha-framework latest version.
webdriverio/webdriverio#2787

We need to update wdio version.

Till then, if you want to run the command gulp test-coverage, just edit out the utils.js file in karma.conf.maker.js

Change line: 21 to:

exclude: /(node_modules)|(test)|(integrationExamples)|(build)|polyfill.js|(src\/adapters\/analytics\/ga.js)|(src\/utils.js)/,

[jsnellbaker]

Closing this ticket in lieu of #5236 which has all the other related items to support node 12. The test-coverage fixes are part of this other ticket.