search

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

Update tap to the latest 5.7 #6

Closed ariya closed 8 years ago

thlorenz commented 8 years ago

Looks like we need to upgrade to npm 2 for install to work. failure Also let's remove older nodes from the tests.

Could you please update the .travis.yml to:

language: node_js
node_js:
  - "0.10"
  - "0.12"
  - "4"
  - "5"
before_install:
  - npm install -g npm@2

and submit with this PR?

Thanks

ariya commented 8 years ago

Sure, it's a good hygiene anyway to update Node.js/npm versions.

ariya commented 8 years ago

Still failing. I may not have enough time to learn tap right now. I don't mind if someone beats me to it.

ariya commented 8 years ago

According to http://www.node-tap.org/changelog/, looks like there will be some more work involved when upgrading tap from 0.x to any version.

ariya commented 8 years ago

I found the issue: t.end() is now mandatory, even if there is a subtest.

ariya commented 8 years ago

Now the failures are legit: they are caused by some dependencies having its .js file as ES6 module. Right now redeyed only parse scripts and it can't detect modules properly. I'll file a separate enhancement issue for that.

ariya commented 8 years ago

I was wrong, the module parsing is a red herring. The problem is that there are too many *.js files (this is recursively searched in node_modules after all) which do not have any var declaration (the testing reference inside redeyed-smoke). Those files need to excluded from the test for the test to succeed. I'll see if I can hunt most of the files for the exclusion.

In the long run, we need to reconsider the value of such a smoke test. It highly depends on other modules directly and indirectly installed by the dependencies (tap and others). It is also a moving target cause the dependencies are not strictly locked, only via ~ version comparator.

ariya commented 8 years ago

@thlorenz This should be good to go now!

ariya commented 8 years ago

@thlorenz Would you like to review this first and shall I merge it?

ariya commented 8 years ago

Since there is no objection, likely I will merge this soon!