thlorenz / redeyed

Takes JavaScript code, along with a config and returns the original code with tokens wrapped and/or replaced as configured.
MIT License
25 stars 7 forks source link

Fix a potentially failing smoke test #5

Closed ariya closed 8 years ago

ariya commented 8 years ago

Exclude whitespace.js from the smoke test since it contains U+180E which is not a whitespace anymore in the upcoming ES2016. That whitespace.js is part of UglifyJS tests, included from the following module dependency:

└─┬ tap@0.4.13 ├─┬ runforcover@0.0.2 │ └─┬ bunker@0.1.2 │ └─┬ burrito@0.2.12 │ ├── traverse@0.5.2 │ └── uglify-js@1.1.1

ariya commented 8 years ago

Alternatively, we can use an updated tap which does not use the old runforcover anymore. However, seems that there was a breaking change and I couldn't drop in the latest tap 5.7 without fixing the entire test suite.

michaelficarra commented 8 years ago

Not only is it not a whitespace character in ES2015, but it's not a whitespace character in any ECMAScript version. Since ECMAScript relies on a given Unicode version or any newer version, it was no longer a whitespace character as soon as the Unicode version where it changed was released.

mathiasbynens commented 8 years ago

@michaelficarra The Unicode v5.1.0 version requirement used to be hardcoded in the spec for whitespace characters, though — separate from the general Unicode version requirement. Even implementations that already used Unicode v8.0.0 elsewhere still had to consider U+180E as whitespace up until ES2015. See https://github.com/tc39/ecma262/pull/300#issuecomment-181376767 and https://github.com/mathiasbynens/regexpu/issues/7.

michaelficarra commented 8 years ago

Oh, hmm. That's interesting. That was probably a mistake. Thanks for the info @mathiasbynens.

thlorenz commented 8 years ago

seems that there was a breaking change and I couldn't drop in the latest tap 5.7 without fixing the entire test suite.

Fixing test suite would be preferred, what kind of problems did you run into?

ariya commented 8 years ago

@thlorenz I'll create a separate PR for tap upgrade and we all can watch its Travis output :smile_cat:

ariya commented 8 years ago

This is not necessary anymore, superseded by PR #6 (tap update).