[WIP] Fix audit warnings wct browser legacy

[TimvdLippe]

This upgrades several packages with (critical) vulnerabilities. There is one 1 low vulnerability left in stacky > lodash. We maintained stacky (https://github.com/googlearchive/stacky), but it has been unmaintained for 2 years. Moreover, lodash is actually never used in the code in stacky. Fixing this vulnerability therefore is easy: simply remove lodash from stacky, yet the package is no longer updated. Not sure what our options are in that regard.

The following list shows the breaking changes in this PR:

mocha

Mocha updated from 3.4.2 to 5.2.0.
Changelists: https://github.com/mochajs/mocha/releases/tag/v4.0.0 and https://github.com/mochajs/mocha/releases/tag/v5.0.0

The breaking changes include removal of old Node versions.
This is not a problem as we support 8+ which still works.
Moreover, IE8, 9 and 10 are no longer supporter.
Again, we do not support these browsers, so this should not be breaking changes for our users.

Expected breaking change impact: None

Chai

Several API updates were made.
Changelist: https://github.com/chaijs/chai/releases/tag/4.0.0

I ran these changes on the Polymer repository.
Out of all tests, there was 1 usage of sinon.reset().
All other tests were happily passing.

Expected breaking change impact: Low ⚠️

Sinon

Updated from 1.17.1 to 6.0.0.
Changelists: http://sinonjs.org/guides/migrating-to-2.0 and http://sinonjs.org/guides/migrating-to-3.0 and http://sinonjs.org/guides/migrating-to-4.0 and http://sinonjs.org/guides/migrating-to-5.0 and http://sinonjs.org/guides/migrating-to-6.0

There are no user-facing breaking changes for the upgrade to version 6.
The other versions introduce several error-throwing methods and some method removal.
None of these changes were impactful for the Polymer repository.

Expected breaking change impact: (Very) Low ⚠️

Sinon-chai

Updated from 2.10.0 to 3.2.0.
Changelist: https://github.com/domenic/sinon-chai/releases/tag/v3.0.0

The breaking changes include removal of old Node versions.
This is not a problem as we support 8+ which still works.

Expected breaking change impact: None

Lodash

Updated from 3.10.1 to 4.17.10.
Changelist: https://github.com/lodash/lodash/wiki/Changelog#compatibility-warnings

These changes include quite some updates to methods.
However, Polymer core does not make use it and I was unable to find other usages of Lodash in our organisations: Polymer and PolymerElements.
(There are usages, but they are in node_modules fixtures of modulizer or implementation of our Node tooling)

Expected breaking change impact: (Very) Low ⚠️

Async

Updated from 1.5.2 to 2.6.1.
Changelist: https://github.com/caolan/async/releases/tag/v2.0.0

I was unable to find any usage of this particular package in our test suites.
The Polymer core repository was doing fine without this.
Not sure what the purpose of this package is and what the impact of the method updates is.

Expected breaking change impact: (Very) Low ⚠️

Fixes #418

[TimvdLippe]

Integrations for WCT are failing, but I am unable to reproduce locally.

Sadly, to fix the issues with wct-browser-legacy, I had to update the web-component-tester config, as that appears to generate the browser.js that has to be updated. As such, I had to update the same bower and npm dependencies in web-component-tester to fix wct-browser-legacy.

Right now, the integration test is locally passing, but on CI the multiple-replace test suite is failing. There is nothing particularly different in this integration test compared to the other tests. The failures happen on the assert that makes sure the test fixtures are correctly setup. While locally that works, on CI this throws.

The weird thing is that I have not updated the version of test-fixture, only the other packages. I therefore do not understand why this suddenly would fail. Any pointers would be greatly appreciated!

[hastebrot]

I'm using @polymer/polymer 3.0.2, web-component-tester 6.7.0, and wct-browser-legacy 1.0.1.

I had to add async 1.5.2 and lodash 3.10.1 to the dev dependencies; during test run I received error messages in the browser console stating these dependencies were missing. I guess this is due to me not using yarn install --flat (I use just yarn install instead). I run wct with wct --npm --module-resolution=node.

Looking forward for this change. I have no idea why your CI is failing, though.

[TimvdLippe]

I reverted the Bower changes. Getting closer on fixing the NPM version. However, running into a compilation performance issue. Extracted issue into #538, which is now blocking this PR, as the integration tests are all timing out on this issue.

[TimvdLippe]

With #543 this branch now builds. However, there are 2 integration tests that are failing. These integration tests verify the behavior the polymer-3.x CLI templates. However, they run npm install, which does not take into account the linked dependencies. Therefore, it will run using web-component-tester with the lates version, but as wct-browser-legacy is specified the package.json of the template, it will fetch the wrong version.

This can be resolved by publishing a new version of wct-browser-legacy to the npm registry and install that, but that would require this PR to be merged. I am not sure how I can resolve this situation and make sure it uses any linked dependency upon installation.

Other than that, this PR is ready to review @aomarks @usergenic

[justinfagnani]

@TimvdLippe I don't think we can upgrade lodash from 3.x to 4.x without revving WCT to 7.0. The question for breaking changes is not just whether it breaks Polymer tests, but whether it would break our other users tests. Lodash 4 has a pretty large number of breaking changes.

This might apply to other upgrades as well, but the lodash breakages are the ones I'm most familiar with.

[TimvdLippe]

@justinfagnani Yes, WCT 7 is inherent with this change. The impact analysis was mostly how much it would bother our users. E.g. if this change would even make sense if the upgrade is doable. If the impact would be large, an upgrade would just be infeasible.

Luckily, while there are breakages, resolving should be doable. Therefore, I think this is feasible :)

[justinfagnani]

@TimvdLippe if we do a breaking change to WCT, I'd really rather we just drop all of the implicit dependencies and require that tests actually import the packages that they use.

@usergenic is working on a new wct-browser package that is essentially this - it hooks up to Mocha for reporting, but requires that tests depend on and import any other packages, including Mocha, themselves. Then we don't have to worry about choosing packages and versions for tests.

The only way I'd want to release a new version of wct-browser-legacy is if it's deprecated upon release, and it's at the same time as a release of a clean wct-browser. This would only be for users to have a slightly easier upgrade path to packages without security warnings, but the recommendation would be that they don't upgrade wct-browser-legacy, but switch to wct-browser.

[TimvdLippe]

I'd really rather we just drop all of the implicit dependencies and require that tests actually import the packages that they use.

Yes absolutely. The more I worked on this, the more I realized it's the wrong approach.

The only way I'd want to release a new version of wct-browser-legacy is if it's deprecated upon release, and it's at the same time as a release of a clean wct-browser.

I think this is a good idea. wct and wct-browser-legacy both ship separate instances. I would therefore propose that we remove all the runner dependency injection logic from wct. Then the new version of WCT demands legacy for all existing users. And then we can state that wct-browser is the way to go, which is essentially the runner code, but then extracted and not coupled to dependency versions.

The upgrade path would therefore be:

1. legacy:1 -> legacy:2 or wct:6 -> legacy:2
2. legacy:2 -> wct-browser:1

WDYT?

[hastebrot]

How will this affect wct? Will the wct-* plugins still work? On the other side: Will it allow us to use custom reporters for Mocha like mochawesome (https://github.com/adamgruber/mochawesome)?

I'd like to create a compatibility list/matrix for wct + wct-browser-legacy, and wct + wct-browser (or whatever approach you'll choose for the old and new dependencies).

[TimvdLippe]

@hastebrot That should be unaffected. We are just bumping the dependency version numbers

[hastebrot]

@TimvdLippe Understood.

I see you removed wct-local and wct-sauce from devDependencies in packages/web-component-tester/package.json. They are now just optionalDependencies. This would allow to use yarn install --ignore-optional. I'd add wct-local to my project's devDependencies and so be able to get rid of wct-sauce, which I doesn't use anyway (and which takes some time to install).

[warpech]

Can anything be done to help with this PR?

[TimvdLippe]

@usergenic I think we can close this PR as superseded by #702, no?

[keanulee]

Superseded by some recent PRs. Upgrading lodash 3 would be a breaking change because it's actually injected into the response when running tests, and it's possible for a user to have test code that uses lodash. Thus, we can't remove the "low" vulnerability without a breaking release.