🏗 Update packages used in validator and validator/nodejs

[rsimha]

The packages used in validator and validator/nodejs have gone out of date. The following security / deprecation warnings are seen during the installation step of gulp validator:

npm WARN deprecated gulp-util@3.0.8: gulp-util is deprecated - replace it, following the guidelines at https://medium.com/gulpjs/gulp-util-ca3b1f9f9ac5
npm WARN deprecated minimatch@0.3.0: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm WARN notice [SECURITY] minimatch has 1 high vulnerability. Go here for more details: https://nodesecurity.io/advisories?search=minimatch&version=0.3.0 - Run `npm i npm@latest -g` to upgrade your npm version, and then `npm audit` to get more info.
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN deprecated minimatch@0.3.0: Please update to minimatch 3.0.2 or higher to avoid a RegExp DoS issue
npm notice created a lockfile as package-lock.json. You should commit this file.

This PR does the following:

• Updates the versions of packages used in validator and validator/nodejs
  • Updates google-closure-compiler and google-closure-library from 20160517 to 20180506
• Uploads package-lock.json so that the versions are locked down during testing on Travis
• Moves all the tests in validator_rules_test.js back to validator_test.js
• Fixes several tests that weren't using Jasmine comparison styles
• Skips a couple of failing / flaky tests (like attrs not sorted alphabetically by name) with TODO(#15443)

This increases the number of tests run by gulp validator from 537 to 5425

[rsimha]

/to @Gregable @honeybadgerdontcare
/cc @erwinmombay

Seems like upgrading google-closure-* from 20160517 to 20180506 has resulted in a change to the order of attributes in validator/dist/validator_test_minified.js. Let me know if this is acceptable.

• If yes, would we need to update the expected output files? Or is there a new flag that must be passed to the closure compiler to revert to the old way of ordering attributes?
• If no, is there a different way in which we can preserve the minified output, while getting rid of the security warnings in the tooling? (See PR description.)

[honeybadgerdontcare]

@rsimha why would this change the order of attributes in the protoascii?

[honeybadgerdontcare]

Ahh, this is the minified version, so closure is doing something odd to the protoasciis. we only care about the ordering of the attributes for the non-minified version, maybe we can move these tests to the non-minified version.

[rsimha]

@honeybadgerdontcare Can do. Are the minified tests in a different file? Or a different section of validator_test.js?

Edit: I just realized that four kinds of tests are failing with the new version of google-closure-compiler:

1. attrs not sorted alphabetically by name: 89 failures
2. a cdata spec must be defined: 7 failures
3. Some error codes are missing format strings: 1 failure
4. Some error codes are missing specificity rules: 1 failure

Let me know if these are expected, and if not, how they can be mitigated.

[honeybadgerdontcare]

Let me take a closer look on Monday. I think we want all these tests against the un-minified protoascii.

[rsimha]

Blocked by #15509

[rsimha]

@honeybadgerdontcare I've updated this PR to run validator_rules_test.js on an unminified build (created by running closure compiler with --compilation_level=WHITESPACE_ONLY). However, when the test runs, I see the following error:

[[build.py RunTests]] - entering ...
/Users/rsimha/src/amphtml/validator/dist/validator_test_unminified.js:92
var EMPTY_amp_validator_ErrorCategory_Code_ARRAY=[];var EMPTY_amp_validator_ExtensionSpec_ExtensionUsageRequirement_ARRAY=[];var EMPTY_amp_validator_HtmlFormat_Code_ARRAY=[];var EMPTY_amp_validator_ValidationError_Code_ARRAY=[];var EMPTY_amp_validator_ValidationError_Severity_ARRAY=[];var EMPTY_amp_validator_ValidationResult_Status_ARRAY=[];amp.validator.AmpLayout=function(supportedLayouts){this.supportedLayouts=supportedLayouts;this.definesDefaultWidth=false;this.definesDefaultHeight=false};
                                                                                                                                                                                                                                                                                                                                                       ^

ReferenceError: amp is not defined
    at Object.<anonymous> (/Users/rsimha/src/amphtml/validator/dist/validator_test_unminified.js:92:344)

Any idea what's missing?

[rsimha]

/cc @erwinmombay, who might know what's going on in #15443 (comment)

[rsimha]

To see why the ReferenceError was being thrown, I generated a manifest file in minified and unminified modes by passing --output_manifest to closure compiler. I'm seeing a difference in the modules being included in minified vs. unminified mode. At this point, I'm out of my depth with validator, so let me know if you spot what's missing. The updated validator/build.py code is part of this PR.

dist/validator_rules_test_unminified.js.manifest:

node_modules/google-closure-library/closure/goog/base.js
node_modules/google-closure-library/closure/goog/deps.js
node_modules/google-closure-library/closure/goog/transitionalforwarddeclarations.js
node_modules/google-closure-library/closure/goog/demos/tree/testdata.js
node_modules/google-closure-library/closure/goog/demos/graphics/tigerdata.js
node_modules/google-closure-library/closure/goog/demos/editor/deps.js
node_modules/google-closure-library/closure/bin/generate_closure_unit_tests/generate_closure_unit_tests.js
node_modules/google-closure-library/third_party/closure/goog/deps.js
node_modules/google-closure-library/third_party/closure/goog/base.js
dist/validator-proto-generated.js
dist/validator-generated.js
engine/validator_rules_test.js

dist/validator_test_minified.js.manifest:

node_modules/google-closure-library/closure/goog/base.js
node_modules/google-closure-library/closure/goog/debug/error.js
node_modules/google-closure-library/closure/goog/dom/nodetype.js
node_modules/google-closure-library/closure/goog/asserts/asserts.js
node_modules/google-closure-library/closure/goog/array/array.js
engine/htmlparser-interface.js
engine/htmlparser.js
dist/validator-proto-generated.js
engine/definitions.js
dist/validator-generated.js
node_modules/google-closure-library/closure/goog/string/string.js
node_modules/google-closure-library/closure/goog/uri/utils.js
engine/tokenize-css.js
engine/parse-css.js
engine/amp4ads-parse-css.js
engine/keyframes-parse-css.js
node_modules/google-closure-library/closure/goog/object/object.js
node_modules/google-closure-library/closure/goog/structs/structs.js
node_modules/google-closure-library/closure/goog/structs/collection.js
node_modules/google-closure-library/closure/goog/functions/functions.js
node_modules/google-closure-library/closure/goog/math/math.js
node_modules/google-closure-library/closure/goog/iter/iter.js
node_modules/google-closure-library/closure/goog/structs/map.js
node_modules/google-closure-library/closure/goog/structs/set.js
engine/parse-srcset.js
engine/parse-url.js
engine/validator.js
engine/validator_test.js

[honeybadgerdontcare]

@rsimha I'm going to guess that validator_rules_test.js still needs validator.js. Note that validator_gen_js.py is what provides these requires in validator_rules_test.js:

goog.require('amp.validator.HtmlFormat');
goog.require('amp.validator.ValidationError');
goog.require('amp.validator.createRules');

[honeybadgerdontcare]

Oh, unless dist/validator-generated.js provides them...I'm also a bit out of my depth on the validator/closure build process.

[rsimha]

@honeybadgerdontcare Yeah, I'm not sure what's going on here. In case someone else knows what's missing, here's how I'm invoking closure compiler (with minify=False).

[rsimha]

@honeybadgerdontcare @Gregable I have some new info after chatting offline with @erwinmombay. It turns out that with the way validator_rules_test.js and validator.js are written today, there's no way to compile them into unminified code using browserify and babelify, since they use closure annotations like goog.provide and goog.require. It will take a non-trivial amount of work to convert the code to work this way, and I'm not sure if that's worth doing.

Since the problem I'm trying to solve right now is the few tests that are failing, particularly the "order of attributes" test, I wanted to propose this plan:

• Upgrade closure compiler from a 2 year old version to a recent version (thereby fixing the security / deprecation warnings)
• Build validator_rules_test.js in minified mode, just like we are doing with validator_tests.js
• Change the "attrs not alphabetized" test to check for the existence of all the attributes in minified code, but ignore the actual order (with a TODO to see if there's a way we can test for order)

WDYT? Is this acceptable for now? (given that we're not running any tests in validator_rules_test.js today)

[Gregable]

That sounds OK to me.

[honeybadgerdontcare]

@rsimha The only reason we're not running those tests is because they were split out as a result of these changes. So I don't view that as a good reason to start ignoring tests.

I suggest we undo the split and not run the attributes in order test with the TODO.

[rsimha]

@Gregable @honeybadgerdontcare @twifkak This is now ready for review. Closure compiler updated, several tests fixed, and one test skipped (with a TODO).

[rsimha]

@honeybadgerdontcare Can do. I assume I should just move the tests back to validator_test.js? Commit coming up.

[honeybadgerdontcare]

@rsimha yes that is where it split from. thx!

[rsimha]

@honeybadgerdontcare Done in final commit. Verified that we're still running the same number of tests. Thanks for all the patient reviews!