Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Node v10 #24

Closed
roback opened this issue Feb 14, 2018 · 4 comments · Fixed by #30
Closed

Add support for Node v10 #24

roback opened this issue Feb 14, 2018 · 4 comments · Fixed by #30
Labels

Comments

@roback
Copy link
Member

roback commented Feb 14, 2018

The following specs fails for unknown reasons when running them with Node v9.5.0. I'm not sure whether this just has to do with the tests or if it is something else in the API client that doesn't work in the new Node version.

  1) client #execute_query with invalid api key should throw error on invalid API key:
      Uncaught AssertionError [ERR_ASSERTION]: Mocks not yet satisfied:
GET http://api.twingly.com:443/blog/search/api/v3/search?apikey=wrong&q=something
      + expected - actual
      -false
      +true
      
      at Object.done (node_modules/nock/lib/scope.js:244:12)
      at Cassette.eject (node_modules/nock-vcr/lib/nock-vcr.js:140:29)
      at Object.ejectCassette (node_modules/nock-vcr/lib/nock-vcr.js:171:58)
      at test/test_client.js:70:26
      at lib/parser.js:24:13
      at Parser.<anonymous> (node_modules/xml2js/lib/parser.js:303:18)
      at SAXParser.onclosetag (node_modules/xml2js/lib/parser.js:261:26)
      at emit (node_modules/sax/lib/sax.js:624:35)
      at emitNode (node_modules/sax/lib/sax.js:629:5)
      at closeTag (node_modules/sax/lib/sax.js:889:7)
      at SAXParser.write (node_modules/sax/lib/sax.js:1436:13)
      at Parser.exports.Parser.Parser.parseString (node_modules/xml2js/lib/parser.js:322:31)
      at Parser.parseString (node_modules/xml2js/lib/parser.js:5:59)
      at exports.parseString (node_modules/xml2js/lib/parser.js:354:19)
      at Parser.parse (lib/parser.js:20:5)
      at lib/client.js:67:24
      at IncomingMessage.<anonymous> (lib/client.js:113:17)
      at endReadableNT (_stream_readable.js:1101:12)
      at process._tickCallback (internal/process/next_tick.js:152:19)
  2) query #execute() when searching for spotify should get posts:
      Uncaught AssertionError [ERR_ASSERTION]: Mocks not yet satisfied:
GET http://api.twingly.com:443/blog/search/api/v3/search?apikey=test-key&q=spotify%20page-size%3A10%20lang%3Asv
      + expected - actual
      -false
      +true
      
      at Object.done (node_modules/nock/lib/scope.js:244:12)
      at Cassette.eject (node_modules/nock-vcr/lib/nock-vcr.js:140:29)
      at Object.ejectCassette (node_modules/nock-vcr/lib/nock-vcr.js:171:58)
      at test/test_query.js:126:26
      at lib/parser.js:24:13
      at Parser.<anonymous> (node_modules/xml2js/lib/parser.js:303:18)
      at SAXParser.onclosetag (node_modules/xml2js/lib/parser.js:261:26)
      at emit (node_modules/sax/lib/sax.js:624:35)
      at emitNode (node_modules/sax/lib/sax.js:629:5)
      at closeTag (node_modules/sax/lib/sax.js:889:7)
      at SAXParser.write (node_modules/sax/lib/sax.js:1436:13)
      at Parser.exports.Parser.Parser.parseString (node_modules/xml2js/lib/parser.js:322:31)
      at Parser.parseString (node_modules/xml2js/lib/parser.js:5:59)
      at exports.parseString (node_modules/xml2js/lib/parser.js:354:19)
      at Parser.parse (lib/parser.js:20:5)
      at lib/client.js:67:24
      at IncomingMessage.<anonymous> (lib/client.js:113:17)
      at endReadableNT (_stream_readable.js:1101:12)
      at process._tickCallback (internal/process/next_tick.js:152:19)

(This was discovered in #23)

@roback roback added the bug label Feb 14, 2018
roback added a commit that referenced this issue Feb 15, 2018
I'm not sure this works or if you have to specify an exact version
for "allow_failures". Anyway, this is just a temporary fix until #24 has
been solved.

Also cleaned up the version we test against:
* 5.x and 7.x removed as their EOL was at 2016-06-30
* 8.x is the current active LTS release

(see https://github.com/nodejs/Release)
@dentarg
Copy link
Contributor

dentarg commented Feb 15, 2018

We could do #17 and see if this is still an issue :)

@roback
Copy link
Member Author

roback commented Feb 15, 2018

Running the tests with Node v8.9.4 records fixtures correctly, but running with v9.5.0 does not record anything.

NOCK_VCR_MODE=ALL npm test

NOCK_VCR_MODE=ALL - always record new fixtures, see nock-vcr README

Fixture after running with Node v8.9.4

// File: test/cassettes/search_without_valid_api_key.js

module.exports = exports = function(nock) {
var refs = [];

refs[0] = nock('http://api.twingly.com:443')
  .get('/blog/search/api/v3/search?apikey=wrong&q=something')
  .reply(401, "<?xml version=\"1.0\" encoding=\"utf-8\"?><error code=\"40101\"><message>Unauthorized</message></error>", { server: 'nginx',
  date: 'Thu, 15 Feb 2018 15:23:25 GMT',
  'content-type': 'application/xml; charset=utf-8',
  'content-length': '97',
  connection: 'close',
  'cache-control': 'no-cache',
  pragma: 'no-cache',
  expires: '-1' });


return refs;
};

Fixture after running with Node v9.5.0

// File: test/cassettes/search_without_valid_api_key.js

module.exports = exports = function(nock) {
var refs = [];


return refs;
};

It looks like there's some changes in the new node version that's not compatible with nock-vcr.

@roback
Copy link
Member Author

roback commented Feb 15, 2018

I also discovered that you can make the tests pass on 9.5.0 by manually editing http to https in the already recorded fixtures...

@roback roback changed the title Add support for Node v9 Add support for Node v10 Aug 27, 2018
@roback
Copy link
Member Author

roback commented Aug 27, 2018

Updated the title since Node v9 is EOL since 2018-06-30. The current release is v10.

Source: https://github.com/nodejs/Release

roback added a commit that referenced this issue Aug 27, 2018
Also removed v4 from the build matrix since it's EOL was at 2018-04-30.
(https://github.com/nodejs/Release#end-of-life-releases)

close #24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants