From a5062586091d8884c0ac48f30594eb2ee74f0b0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Sat, 20 Jun 2020 11:21:00 +0300 Subject: [PATCH 01/19] test: fix test failure introduced due to PRs not being in sync with each other --- test/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/index.js b/test/index.js index 60e72a4..ced0d70 100644 --- a/test/index.js +++ b/test/index.js @@ -456,7 +456,7 @@ describe('detect-node-support', () => { it('supports "owner/repo" style repository string', async () => { - listRemoteStub + fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); Nock('https://raw.githubusercontent.com') @@ -467,8 +467,8 @@ describe('detect-node-support', () => { const result = await NodeSupport.detect({ repository: 'pkgjs/detect-node-support' }); - expect(listRemoteStub.callCount).to.equal(1); - expect(listRemoteStub.args[0]).to.equal([['http://github.com/pkgjs/detect-node-support', 'HEAD']]); + expect(fixture.stubs.listRemote.callCount).to.equal(1); + expect(fixture.stubs.listRemote.args[0]).to.equal([['http://github.com/pkgjs/detect-node-support', 'HEAD']]); expect(result).to.equal({ name: 'detect-node-support', From 8aeb25817bf2fd09479b862535f28eb8d0546a3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Thu, 28 May 2020 17:58:24 +0300 Subject: [PATCH 02/19] feat: basic importing --- lib/travis/index.js | 45 ++++++++++++- test/fixtures/index.js | 9 ++- .../testing-imports/partials/node-14.yml | 2 + .../testing-imports/simple-array-object.yml | 3 + .../testing-imports/simple-array-string.yml | 3 + .../testing-imports/simple-single-object.yml | 3 + .../testing-imports/simple-single-string.yml | 2 + test/travis.js | 65 +++++++++++++++++++ 8 files changed, 128 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/travis-ymls/testing-imports/partials/node-14.yml create mode 100644 test/fixtures/travis-ymls/testing-imports/simple-array-object.yml create mode 100644 test/fixtures/travis-ymls/testing-imports/simple-array-string.yml create mode 100644 test/fixtures/travis-ymls/testing-imports/simple-single-object.yml create mode 100644 test/fixtures/travis-ymls/testing-imports/simple-single-string.yml create mode 100644 test/travis.js diff --git a/lib/travis/index.js b/lib/travis/index.js index 937fc29..e9b66cf 100644 --- a/lib/travis/index.js +++ b/lib/travis/index.js @@ -23,7 +23,46 @@ internals.toArray = (v) => { }; -internals.scan = async (travisYaml) => { +internals.normalizeImports = (travisYaml) => { + + return internals.toArray(travisYaml.import).map((entry) => { + + if (typeof entry === 'string') { + entry = { source: entry }; + } + + return entry; + }); +}; + + +internals.applyImports = async (yaml, { loadFile }) => { + + if (!yaml.import) { + return; + } + + const imports = internals.normalizeImports(yaml); + + for (const entry of imports) { + + const buffer = await loadFile(entry.source); + + const imported = Yaml.safeLoad(buffer, { + schema: Yaml.FAILSAFE_SCHEMA, + json: true + }); + + for (const key in imported) { + yaml[key] = imported[key]; + } + } +}; + + +internals.scan = async (travisYaml, options) => { + + await internals.applyImports(travisYaml, options); const rawSet = new Set(); @@ -35,7 +74,7 @@ internals.scan = async (travisYaml) => { for (const env of internals.toArray(travisYaml.env.matrix)) { - const matches = env.match(/(?:NODEJS_VER|TRAVIS_NODE_VERSION|NODE_VER)="?(node\/)?(?[\w./*]+)"?/); /* hack syntax highlighter 🤦‍♂️ */ + const matches = env.match(/(?:NODEJS_VER|TRAVIS_NODE_VERSION|NODE_VER)="?(node\/)?(?[\w./*]+)"?/); if (matches) { rawSet.add(matches.groups.version); @@ -96,6 +135,6 @@ exports.detect = async ({ loadFile }) => { }); return { - travis: await internals.scan(travisYaml) + travis: await internals.scan(travisYaml, { loadFile }) }; }; diff --git a/test/fixtures/index.js b/test/fixtures/index.js index ccf142e..e882b1e 100644 --- a/test/fixtures/index.js +++ b/test/fixtures/index.js @@ -75,7 +75,7 @@ module.exports = class TestContext { }); } - async setupRepoFolder({ travisYml, packageJson, npmShrinkwrapJson, packageLockJson, git = true } = {}) { + async setupRepoFolder({ travisYml, partials, packageJson, npmShrinkwrapJson, packageLockJson, git = true } = {}) { const tmpObj = Tmp.dirSync({ unsafeCleanup: true }); @@ -87,6 +87,13 @@ module.exports = class TestContext { Fs.copyFileSync(Path.join(__dirname, 'travis-ymls', travisYml), Path.join(this.path, '.travis.yml')); } + if (partials) { + Fs.mkdirSync(Path.join(this.path, 'partials')); + for (const fn of ['node-14.yml']) { + Fs.copyFileSync(Path.join(__dirname, 'travis-ymls', 'testing-imports', 'partials', fn), Path.join(this.path, 'partials', fn)); + } + } + if (packageJson !== false) { Fs.writeFileSync(Path.join(this.path, 'package.json'), JSON.stringify(packageJson || { name: 'test-module', diff --git a/test/fixtures/travis-ymls/testing-imports/partials/node-14.yml b/test/fixtures/travis-ymls/testing-imports/partials/node-14.yml new file mode 100644 index 0000000..4af9da1 --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/partials/node-14.yml @@ -0,0 +1,2 @@ +node_js: + - "14" diff --git a/test/fixtures/travis-ymls/testing-imports/simple-array-object.yml b/test/fixtures/travis-ymls/testing-imports/simple-array-object.yml new file mode 100644 index 0000000..66891bf --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/simple-array-object.yml @@ -0,0 +1,3 @@ +language: node_js +import: + - source: partials/node-14.yml diff --git a/test/fixtures/travis-ymls/testing-imports/simple-array-string.yml b/test/fixtures/travis-ymls/testing-imports/simple-array-string.yml new file mode 100644 index 0000000..5284f7f --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/simple-array-string.yml @@ -0,0 +1,3 @@ +language: node_js +import: + - partials/node-14.yml diff --git a/test/fixtures/travis-ymls/testing-imports/simple-single-object.yml b/test/fixtures/travis-ymls/testing-imports/simple-single-object.yml new file mode 100644 index 0000000..6fe1c48 --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/simple-single-object.yml @@ -0,0 +1,3 @@ +language: node_js +import: + source: partials/node-14.yml diff --git a/test/fixtures/travis-ymls/testing-imports/simple-single-string.yml b/test/fixtures/travis-ymls/testing-imports/simple-single-string.yml new file mode 100644 index 0000000..f7877b4 --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/simple-single-string.yml @@ -0,0 +1,2 @@ +language: node_js +import: partials/node-14.yml diff --git a/test/travis.js b/test/travis.js new file mode 100644 index 0000000..281c1a4 --- /dev/null +++ b/test/travis.js @@ -0,0 +1,65 @@ +'use strict'; + +const NodeSupport = require('..'); + +const TestContext = require('./fixtures'); + + +const { describe, it, beforeEach, afterEach } = exports.lab = require('@hapi/lab').script(); +const { expect } = require('@hapi/code'); + + +const internals = {}; + + +internals.assertCommit = (result) => { + + expect(result.commit).to.match(/^[0-9a-f]{40}$/); + delete result.commit; +}; + + +describe('.travis.yml parsing', () => { + + let fixture; + + beforeEach(() => { + + fixture = new TestContext(); + }); + + afterEach(() => { + + fixture.cleanup(); + }); + + describe('simple', () => { + + for (const yml of ['simple-single-object', 'simple-single-string', 'simple-array-object', 'simple-array-string']) { + + // eslint-disable-next-line no-loop-func + it(`resolves simple local import (${yml})`, async () => { + + await fixture.setupRepoFolder({ + partials: true, + travisYml: `testing-imports/${yml}.yml` + }); + + const result = await NodeSupport.detect({ path: fixture.path }); + + internals.assertCommit(result); + + expect(result).to.equal({ + name: 'test-module', + version: '0.0.0-development', + timestamp: 1580673602000, + travis: { + raw: ['14'], + resolved: { '14': '14.3.0' } + } + }); + }); + } + }); + +}); From 1a2964ab7978fe2a90b98d22fa90c7203bfb5b63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Thu, 28 May 2020 18:21:50 +0300 Subject: [PATCH 03/19] feat: support indirect import --- lib/travis/index.js | 20 +++++- test/fixtures/index.js | 6 +- .../travis-ymls/testing-imports/indirect.yml | 3 + .../partials/indirect-node-14.yml | 2 + test/travis.js | 61 ++++++++++++------- 5 files changed, 67 insertions(+), 25 deletions(-) create mode 100644 test/fixtures/travis-ymls/testing-imports/indirect.yml create mode 100644 test/fixtures/travis-ymls/testing-imports/partials/indirect-node-14.yml diff --git a/lib/travis/index.js b/lib/travis/index.js index e9b66cf..2dea646 100644 --- a/lib/travis/index.js +++ b/lib/travis/index.js @@ -23,7 +23,7 @@ internals.toArray = (v) => { }; -internals.normalizeImports = (travisYaml) => { +internals.normalizeImports = (travisYaml, { relativeTo }) => { return internals.toArray(travisYaml.import).map((entry) => { @@ -31,18 +31,25 @@ internals.normalizeImports = (travisYaml) => { entry = { source: entry }; } + if (entry.source.startsWith('./')) { + const relativeParts = relativeTo.source.split('/'); + relativeParts.pop(); + relativeParts.push(entry.source.substring(2)); + entry.source = relativeParts.join('/'); + } + return entry; }); }; -internals.applyImports = async (yaml, { loadFile }) => { +internals.applyImports = async (yaml, { loadFile, relativeTo }) => { if (!yaml.import) { return; } - const imports = internals.normalizeImports(yaml); + const imports = internals.normalizeImports(yaml, { relativeTo }); for (const entry of imports) { @@ -53,7 +60,14 @@ internals.applyImports = async (yaml, { loadFile }) => { json: true }); + await internals.applyImports(imported, { loadFile, relativeTo: entry }); + for (const key in imported) { + + if (key === 'import') { + continue; + } + yaml[key] = imported[key]; } } diff --git a/test/fixtures/index.js b/test/fixtures/index.js index e882b1e..2dbcde0 100644 --- a/test/fixtures/index.js +++ b/test/fixtures/index.js @@ -89,7 +89,11 @@ module.exports = class TestContext { if (partials) { Fs.mkdirSync(Path.join(this.path, 'partials')); - for (const fn of ['node-14.yml']) { + const partialYmls = [ + 'indirect-node-14.yml', + 'node-14.yml' + ]; + for (const fn of partialYmls) { Fs.copyFileSync(Path.join(__dirname, 'travis-ymls', 'testing-imports', 'partials', fn), Path.join(this.path, 'partials', fn)); } } diff --git a/test/fixtures/travis-ymls/testing-imports/indirect.yml b/test/fixtures/travis-ymls/testing-imports/indirect.yml new file mode 100644 index 0000000..e172b27 --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/indirect.yml @@ -0,0 +1,3 @@ +language: node_js +import: + - source: partials/indirect-node-14.yml diff --git a/test/fixtures/travis-ymls/testing-imports/partials/indirect-node-14.yml b/test/fixtures/travis-ymls/testing-imports/partials/indirect-node-14.yml new file mode 100644 index 0000000..f7b6fed --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/partials/indirect-node-14.yml @@ -0,0 +1,2 @@ +import: + - source: ./node-14.yml diff --git a/test/travis.js b/test/travis.js index 281c1a4..0dfdc46 100644 --- a/test/travis.js +++ b/test/travis.js @@ -33,33 +33,52 @@ describe('.travis.yml parsing', () => { fixture.cleanup(); }); - describe('simple', () => { + for (const yml of ['simple-single-object', 'simple-single-string', 'simple-array-object', 'simple-array-string']) { - for (const yml of ['simple-single-object', 'simple-single-string', 'simple-array-object', 'simple-array-string']) { + // eslint-disable-next-line no-loop-func + it(`resolves simple local import (${yml})`, async () => { - // eslint-disable-next-line no-loop-func - it(`resolves simple local import (${yml})`, async () => { - - await fixture.setupRepoFolder({ - partials: true, - travisYml: `testing-imports/${yml}.yml` - }); + await fixture.setupRepoFolder({ + partials: true, + travisYml: `testing-imports/${yml}.yml` + }); - const result = await NodeSupport.detect({ path: fixture.path }); + const result = await NodeSupport.detect({ path: fixture.path }); - internals.assertCommit(result); + internals.assertCommit(result); - expect(result).to.equal({ - name: 'test-module', - version: '0.0.0-development', - timestamp: 1580673602000, - travis: { - raw: ['14'], - resolved: { '14': '14.3.0' } - } - }); + expect(result).to.equal({ + name: 'test-module', + version: '0.0.0-development', + timestamp: 1580673602000, + travis: { + raw: ['14'], + resolved: { '14': '14.3.0' } + } }); - } + }); + } + + it('resolves indirect imports', async () => { + + await fixture.setupRepoFolder({ + partials: true, + travisYml: `testing-imports/indirect.yml` + }); + + const result = await NodeSupport.detect({ path: fixture.path }); + + internals.assertCommit(result); + + expect(result).to.equal({ + name: 'test-module', + version: '0.0.0-development', + timestamp: 1580673602000, + travis: { + raw: ['14'], + resolved: { '14': '14.3.0' } + } + }); }); }); From 20e2ab1f727d4f0b99f91e21a52a9e673f09ab66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Sun, 14 Jun 2020 20:21:42 +0300 Subject: [PATCH 04/19] feat: resolve indirect relative import at root with a ./ --- lib/travis/index.js | 12 ++++++---- .../testing-imports/indirect-dot-slash.yml | 3 +++ test/travis.js | 22 +++++++++++++++++++ 3 files changed, 33 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/travis-ymls/testing-imports/indirect-dot-slash.yml diff --git a/lib/travis/index.js b/lib/travis/index.js index 2dea646..e4c9330 100644 --- a/lib/travis/index.js +++ b/lib/travis/index.js @@ -32,10 +32,14 @@ internals.normalizeImports = (travisYaml, { relativeTo }) => { } if (entry.source.startsWith('./')) { - const relativeParts = relativeTo.source.split('/'); - relativeParts.pop(); - relativeParts.push(entry.source.substring(2)); - entry.source = relativeParts.join('/'); + entry.source = entry.source.substring(2); + + if (relativeTo) { + const relativeParts = relativeTo.source.split('/'); + relativeParts.pop(); + relativeParts.push(entry.source); + entry.source = relativeParts.join('/'); + } } return entry; diff --git a/test/fixtures/travis-ymls/testing-imports/indirect-dot-slash.yml b/test/fixtures/travis-ymls/testing-imports/indirect-dot-slash.yml new file mode 100644 index 0000000..c36485c --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/indirect-dot-slash.yml @@ -0,0 +1,3 @@ +language: node_js +import: + - source: ./partials/indirect-node-14.yml diff --git a/test/travis.js b/test/travis.js index 0dfdc46..d161341 100644 --- a/test/travis.js +++ b/test/travis.js @@ -81,4 +81,26 @@ describe('.travis.yml parsing', () => { }); }); + it('resolves indirect imports (./)', async () => { + + await fixture.setupRepoFolder({ + partials: true, + travisYml: `testing-imports/indirect-dot-slash.yml` + }); + + const result = await NodeSupport.detect({ path: fixture.path }); + + internals.assertCommit(result); + + expect(result).to.equal({ + name: 'test-module', + version: '0.0.0-development', + timestamp: 1580673602000, + travis: { + raw: ['14'], + resolved: { '14': '14.3.0' } + } + }); + }); + }); From bea72c10a5471523405cd4389f30b3ff5fd7d48d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Sun, 14 Jun 2020 20:28:07 +0300 Subject: [PATCH 05/19] feat: ignore conditional imports --- lib/travis/index.js | 30 ++++++++++--------- .../testing-imports/conditional.yml | 4 +++ test/travis.js | 22 ++++++++++++++ 3 files changed, 42 insertions(+), 14 deletions(-) create mode 100644 test/fixtures/travis-ymls/testing-imports/conditional.yml diff --git a/lib/travis/index.js b/lib/travis/index.js index e4c9330..51897a5 100644 --- a/lib/travis/index.js +++ b/lib/travis/index.js @@ -25,25 +25,27 @@ internals.toArray = (v) => { internals.normalizeImports = (travisYaml, { relativeTo }) => { - return internals.toArray(travisYaml.import).map((entry) => { + return internals.toArray(travisYaml.import) + .map((entry) => { - if (typeof entry === 'string') { - entry = { source: entry }; - } + if (typeof entry === 'string') { + entry = { source: entry }; + } - if (entry.source.startsWith('./')) { - entry.source = entry.source.substring(2); + if (entry.source.startsWith('./')) { + entry.source = entry.source.substring(2); - if (relativeTo) { - const relativeParts = relativeTo.source.split('/'); - relativeParts.pop(); - relativeParts.push(entry.source); - entry.source = relativeParts.join('/'); + if (relativeTo) { + const relativeParts = relativeTo.source.split('/'); + relativeParts.pop(); + relativeParts.push(entry.source); + entry.source = relativeParts.join('/'); + } } - } - return entry; - }); + return entry; + }) + .filter((entry) => !entry.if); // @todo: log a warning }; diff --git a/test/fixtures/travis-ymls/testing-imports/conditional.yml b/test/fixtures/travis-ymls/testing-imports/conditional.yml new file mode 100644 index 0000000..6dbf3fb --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/conditional.yml @@ -0,0 +1,4 @@ +language: node_js +import: + - source: partials/indirect-node-14.yml + if: branch = master diff --git a/test/travis.js b/test/travis.js index d161341..02dc15f 100644 --- a/test/travis.js +++ b/test/travis.js @@ -103,4 +103,26 @@ describe('.travis.yml parsing', () => { }); }); + it('ignores conditional imports', async () => { + + await fixture.setupRepoFolder({ + partials: true, + travisYml: `testing-imports/conditional.yml` + }); + + const result = await NodeSupport.detect({ path: fixture.path }); + + internals.assertCommit(result); + + expect(result).to.equal({ + name: 'test-module', + version: '0.0.0-development', + timestamp: 1580673602000, + travis: { + raw: ['latest'], + resolved: { 'latest': '13.14.0' } + } + }); + }); + }); From a8a1d86e2a4387173330d767e7782cf81f4b24a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Sun, 14 Jun 2020 20:55:48 +0300 Subject: [PATCH 06/19] feat: load imports from another repo --- lib/travis/index.js | 37 +++++++++++++++++-- .../testing-imports/another-repo.yml | 4 ++ test/travis.js | 32 ++++++++++++++++ 3 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 test/fixtures/travis-ymls/testing-imports/another-repo.yml diff --git a/lib/travis/index.js b/lib/travis/index.js index 51897a5..1545ed5 100644 --- a/lib/travis/index.js +++ b/lib/travis/index.js @@ -1,11 +1,18 @@ 'use strict'; +const Debug = require('debug'); const Nv = require('@pkgjs/nv'); const Yaml = require('js-yaml'); +const Loader = require('./loader'); + const internals = {}; + +internals.log = Debug('detect-node-support'); + + internals.nodeAliases = { latest: 'active', node: 'active', @@ -49,7 +56,31 @@ internals.normalizeImports = (travisYaml, { relativeTo }) => { }; -internals.applyImports = async (yaml, { loadFile, relativeTo }) => { +internals.loadSource = async (source, { loadFile, cache }) => { + + if (cache[source]) { + internals.log('Returning cached %s', source); + return cache[source]; + } + + let path = source; + + if (source.includes(':')) { + const [repository, fileName] = source.split(':'); + const loader = await Loader.create({ repository: `https://github.com/${repository}` }); + + path = fileName; + loadFile = loader.loadFile; + } + + internals.log('Loading %s', source); + const result = await loadFile(path); + cache[source] = result; + return result; +}; + + +internals.applyImports = async (yaml, { loadFile, relativeTo, cache = new Map() }) => { if (!yaml.import) { return; @@ -59,14 +90,14 @@ internals.applyImports = async (yaml, { loadFile, relativeTo }) => { for (const entry of imports) { - const buffer = await loadFile(entry.source); + const buffer = await internals.loadSource(entry.source, { loadFile, cache }); const imported = Yaml.safeLoad(buffer, { schema: Yaml.FAILSAFE_SCHEMA, json: true }); - await internals.applyImports(imported, { loadFile, relativeTo: entry }); + await internals.applyImports(imported, { loadFile, relativeTo: entry, cache }); for (const key in imported) { diff --git a/test/fixtures/travis-ymls/testing-imports/another-repo.yml b/test/fixtures/travis-ymls/testing-imports/another-repo.yml new file mode 100644 index 0000000..27068ca --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/another-repo.yml @@ -0,0 +1,4 @@ +language: node_js +import: + - source: pkgjs/detect-node-support:test/fixtures/travis-ymls/testing-imports/partials/indirect-node-14.yml + - source: pkgjs/detect-node-support:test/fixtures/travis-ymls/testing-imports/partials/node-14.yml # cache hit diff --git a/test/travis.js b/test/travis.js index 02dc15f..fb519ca 100644 --- a/test/travis.js +++ b/test/travis.js @@ -1,5 +1,9 @@ 'use strict'; +const Fs = require('fs'); +const Nock = require('nock'); +const Path = require('path'); + const NodeSupport = require('..'); const TestContext = require('./fixtures'); @@ -125,4 +129,32 @@ describe('.travis.yml parsing', () => { }); }); + it('resolves from another repo', async () => { + + Nock('https://raw.githubusercontent.com') + .get('/pkgjs/detect-node-support/HEAD/test/fixtures/travis-ymls/testing-imports/partials/indirect-node-14.yml') + .reply(200, Fs.readFileSync(Path.join(__dirname, 'fixtures', 'travis-ymls', 'testing-imports', 'partials', 'indirect-node-14.yml'))) + .get('/pkgjs/detect-node-support/HEAD/test/fixtures/travis-ymls/testing-imports/partials/node-14.yml') + .reply(200, Fs.readFileSync(Path.join(__dirname, 'fixtures', 'travis-ymls', 'testing-imports', 'partials', 'node-14.yml'))); + + await fixture.setupRepoFolder({ + partials: true, + travisYml: `testing-imports/another-repo.yml` + }); + + const result = await NodeSupport.detect({ path: fixture.path }); + + internals.assertCommit(result); + + expect(result).to.equal({ + name: 'test-module', + version: '0.0.0-development', + timestamp: 1580673602000, + travis: { + raw: ['14'], + resolved: { '14': '14.3.0' } + } + }); + }); + }); From 11329ed2183c33c8ef5a109a9439c59e7ed1b2e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Sun, 14 Jun 2020 21:09:07 +0300 Subject: [PATCH 07/19] refactor: split up travis/index.js --- lib/travis/imports.js | 94 +++++++++++++++++++++++++++++++++++++ lib/travis/index.js | 106 +++--------------------------------------- lib/utils.js | 10 ++++ 3 files changed, 110 insertions(+), 100 deletions(-) create mode 100644 lib/travis/imports.js diff --git a/lib/travis/imports.js b/lib/travis/imports.js new file mode 100644 index 0000000..e55d5e3 --- /dev/null +++ b/lib/travis/imports.js @@ -0,0 +1,94 @@ +'use strict'; + +const Debug = require('debug'); +const Yaml = require('js-yaml'); + +const Loader = require('../loader'); +const Utils = require('../utils'); + + +const internals = {}; + + +internals.log = Debug('detect-node-support'); + + +internals.normalizeImports = (travisYaml, { relativeTo }) => { + + return Utils.toArray(travisYaml.import) + .map((entry) => { + + if (typeof entry === 'string') { + entry = { source: entry }; + } + + if (entry.source.startsWith('./')) { + entry.source = entry.source.substring(2); + + if (relativeTo) { + const relativeParts = relativeTo.source.split('/'); + relativeParts.pop(); + relativeParts.push(entry.source); + entry.source = relativeParts.join('/'); + } + } + + return entry; + }) + .filter((entry) => !entry.if); // @todo: log a warning +}; + + +internals.loadSource = async (source, { loadFile, cache }) => { + + if (cache[source]) { + internals.log('Returning cached %s', source); + return cache[source]; + } + + let path = source; + + if (source.includes(':')) { + const [repository, fileName] = source.split(':'); + const loader = await Loader.create({ repository: `https://github.com/${repository}` }); + + path = fileName; + loadFile = loader.loadFile; + } + + internals.log('Loading %s', source); + const result = await loadFile(path); + cache[source] = result; + return result; +}; + + +exports.apply = async (yaml, { loadFile, relativeTo, cache = new Map() }) => { + + if (!yaml.import) { + return; + } + + const imports = internals.normalizeImports(yaml, { relativeTo }); + + for (const entry of imports) { + + const buffer = await internals.loadSource(entry.source, { loadFile, cache }); + + const imported = Yaml.safeLoad(buffer, { + schema: Yaml.FAILSAFE_SCHEMA, + json: true + }); + + await exports.apply(imported, { loadFile, relativeTo: entry, cache }); + + for (const key in imported) { + + if (key === 'import') { + continue; + } + + yaml[key] = imported[key]; + } + } +}; diff --git a/lib/travis/index.js b/lib/travis/index.js index 1545ed5..9d0e3d9 100644 --- a/lib/travis/index.js +++ b/lib/travis/index.js @@ -1,18 +1,15 @@ 'use strict'; -const Debug = require('debug'); const Nv = require('@pkgjs/nv'); const Yaml = require('js-yaml'); -const Loader = require('./loader'); +const TravisImports = require('./imports'); +const Utils = require('../utils'); const internals = {}; -internals.log = Debug('detect-node-support'); - - internals.nodeAliases = { latest: 'active', node: 'active', @@ -20,110 +17,19 @@ internals.nodeAliases = { }; -internals.toArray = (v) => { - - if (v === undefined) { - return []; - } - - return Array.isArray(v) ? v : [v]; -}; - - -internals.normalizeImports = (travisYaml, { relativeTo }) => { - - return internals.toArray(travisYaml.import) - .map((entry) => { - - if (typeof entry === 'string') { - entry = { source: entry }; - } - - if (entry.source.startsWith('./')) { - entry.source = entry.source.substring(2); - - if (relativeTo) { - const relativeParts = relativeTo.source.split('/'); - relativeParts.pop(); - relativeParts.push(entry.source); - entry.source = relativeParts.join('/'); - } - } - - return entry; - }) - .filter((entry) => !entry.if); // @todo: log a warning -}; - - -internals.loadSource = async (source, { loadFile, cache }) => { - - if (cache[source]) { - internals.log('Returning cached %s', source); - return cache[source]; - } - - let path = source; - - if (source.includes(':')) { - const [repository, fileName] = source.split(':'); - const loader = await Loader.create({ repository: `https://github.com/${repository}` }); - - path = fileName; - loadFile = loader.loadFile; - } - - internals.log('Loading %s', source); - const result = await loadFile(path); - cache[source] = result; - return result; -}; - - -internals.applyImports = async (yaml, { loadFile, relativeTo, cache = new Map() }) => { - - if (!yaml.import) { - return; - } - - const imports = internals.normalizeImports(yaml, { relativeTo }); - - for (const entry of imports) { - - const buffer = await internals.loadSource(entry.source, { loadFile, cache }); - - const imported = Yaml.safeLoad(buffer, { - schema: Yaml.FAILSAFE_SCHEMA, - json: true - }); - - await internals.applyImports(imported, { loadFile, relativeTo: entry, cache }); - - for (const key in imported) { - - if (key === 'import') { - continue; - } - - yaml[key] = imported[key]; - } - } -}; - - internals.scan = async (travisYaml, options) => { - await internals.applyImports(travisYaml, options); + await TravisImports.apply(travisYaml, options); const rawSet = new Set(); - for (const v of internals.toArray(travisYaml.node_js)) { + for (const v of Utils.toArray(travisYaml.node_js)) { rawSet.add(v); } if (travisYaml.env) { - for (const env of internals.toArray(travisYaml.env.matrix)) { + for (const env of Utils.toArray(travisYaml.env.matrix)) { const matches = env.match(/(?:NODEJS_VER|TRAVIS_NODE_VERSION|NODE_VER)="?(node\/)?(?[\w./*]+)"?/); @@ -135,7 +41,7 @@ internals.scan = async (travisYaml, options) => { if (travisYaml.matrix) { - for (const include of internals.toArray(travisYaml.matrix.include)) { + for (const include of Utils.toArray(travisYaml.matrix.include)) { if (include.node_js) { rawSet.add(include.node_js); diff --git a/lib/utils.js b/lib/utils.js index 07960ac..63fcb68 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -22,3 +22,13 @@ exports.getErrorMessage = (error) => { return null; }; + + +exports.toArray = (v) => { + + if (v === undefined) { + return []; + } + + return Array.isArray(v) ? v : [v]; +}; From f62fad386fc67d4e2189241af1dfed67a6c9e69a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Mon, 15 Jun 2020 00:02:55 +0300 Subject: [PATCH 08/19] feat: implement merge modes --- lib/travis/imports.js | 25 +- lib/travis/merge.js | 65 +++++ test/fixtures/index.js | 3 + .../merge-deep-prepend-append.yml | 9 + .../testing-imports/merge-deep.yml | 7 + .../testing-imports/merge-invalid.yml | 5 + .../testing-imports/merge-shallow.yml | 7 + .../partials/merge-invalid.yml | 7 + .../testing-imports/partials/node-10.yml | 3 + .../testing-imports/partials/node-12.yml | 2 + test/travis.js | 231 ++++++++++++++++++ 11 files changed, 356 insertions(+), 8 deletions(-) create mode 100644 lib/travis/merge.js create mode 100644 test/fixtures/travis-ymls/testing-imports/merge-deep-prepend-append.yml create mode 100644 test/fixtures/travis-ymls/testing-imports/merge-deep.yml create mode 100644 test/fixtures/travis-ymls/testing-imports/merge-invalid.yml create mode 100644 test/fixtures/travis-ymls/testing-imports/merge-shallow.yml create mode 100644 test/fixtures/travis-ymls/testing-imports/partials/merge-invalid.yml create mode 100644 test/fixtures/travis-ymls/testing-imports/partials/node-10.yml create mode 100644 test/fixtures/travis-ymls/testing-imports/partials/node-12.yml diff --git a/lib/travis/imports.js b/lib/travis/imports.js index e55d5e3..3f7b6d4 100644 --- a/lib/travis/imports.js +++ b/lib/travis/imports.js @@ -6,8 +6,12 @@ const Yaml = require('js-yaml'); const Loader = require('../loader'); const Utils = require('../utils'); +const TravisMerge = require('./merge'); -const internals = {}; + +const internals = { + validMergeModes: new Set(['deep_merge_append', 'deep_merge_prepend', 'deep_merge', 'merge']) +}; internals.log = Debug('detect-node-support'); @@ -22,6 +26,8 @@ internals.normalizeImports = (travisYaml, { relativeTo }) => { entry = { source: entry }; } + const original = entry.source; + if (entry.source.startsWith('./')) { entry.source = entry.source.substring(2); @@ -33,6 +39,14 @@ internals.normalizeImports = (travisYaml, { relativeTo }) => { } } + if (!entry.mode) { + entry.mode = 'deep_merge_append'; + } + + if (!internals.validMergeModes.has(entry.mode)) { + throw new Error(`Invalid merge mode for ${original} in ${relativeTo ? relativeTo.source : '.travis.yml'}: ${entry.mode}`); + } + return entry; }) .filter((entry) => !entry.if); // @todo: log a warning @@ -82,13 +96,8 @@ exports.apply = async (yaml, { loadFile, relativeTo, cache = new Map() }) => { await exports.apply(imported, { loadFile, relativeTo: entry, cache }); - for (const key in imported) { - - if (key === 'import') { - continue; - } + delete imported.import; - yaml[key] = imported[key]; - } + TravisMerge[entry.mode](yaml, imported); } }; diff --git a/lib/travis/merge.js b/lib/travis/merge.js new file mode 100644 index 0000000..6c66fcb --- /dev/null +++ b/lib/travis/merge.js @@ -0,0 +1,65 @@ +'use strict'; + +// ref: https://github.com/travis-ci/travis-yml/blob/bf82881491134c72a64778f9664a8dd3f97158e7/lib/travis/yml/support/merge.rb + +const internals = {}; + + +internals.isObject = (arg) => typeof arg === 'object' && !Array.isArray(arg); + + +exports.deep_merge_append = (left, right) => { + + for (const key in right) { + + if (internals.isObject(left[key]) && internals.isObject(right[key])) { + exports.deep_merge_append(left[key], right[key]); + continue; + } + + if (Array.isArray(left[key]) && Array.isArray(right[key])) { + left[key].push(...right[key]); + continue; + } + + left[key] = right[key]; + } +}; + +exports.deep_merge_prepend = (left, right) => { + + for (const key in right) { + + if (internals.isObject(left[key]) && internals.isObject(right[key])) { + exports.deep_merge_prepend(left[key], right[key]); + continue; + } + + if (Array.isArray(left[key]) && Array.isArray(right[key])) { + left[key].unshift(...right[key]); + continue; + } + + left[key] = right[key]; + } +}; + +exports.deep_merge = (left, right) => { + + for (const key in right) { + + if (internals.isObject(left[key]) && internals.isObject(right[key])) { + exports.deep_merge(left[key], right[key]); + continue; + } + + left[key] = right[key]; + } +}; + +exports.merge = (left, right) => { + + for (const key in right) { + left[key] = right[key]; + } +}; diff --git a/test/fixtures/index.js b/test/fixtures/index.js index 2dbcde0..f71f9b7 100644 --- a/test/fixtures/index.js +++ b/test/fixtures/index.js @@ -91,6 +91,9 @@ module.exports = class TestContext { Fs.mkdirSync(Path.join(this.path, 'partials')); const partialYmls = [ 'indirect-node-14.yml', + 'merge-invalid.yml', + 'node-10.yml', + 'node-12.yml', 'node-14.yml' ]; for (const fn of partialYmls) { diff --git a/test/fixtures/travis-ymls/testing-imports/merge-deep-prepend-append.yml b/test/fixtures/travis-ymls/testing-imports/merge-deep-prepend-append.yml new file mode 100644 index 0000000..1ab7a15 --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/merge-deep-prepend-append.yml @@ -0,0 +1,9 @@ +language: node_js +node_js: + - "8" +import: + - source: partials/node-14.yml # default merge: deep_merge_append + - source: partials/node-12.yml + mode: deep_merge_prepend + - source: partials/node-10.yml + mode: deep_merge_append diff --git a/test/fixtures/travis-ymls/testing-imports/merge-deep.yml b/test/fixtures/travis-ymls/testing-imports/merge-deep.yml new file mode 100644 index 0000000..bc8e3d5 --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/merge-deep.yml @@ -0,0 +1,7 @@ +language: node_js +node_js: + - "8" +import: + - source: partials/node-14.yml # default merge: deep_merge_append + - source: partials/node-12.yml + mode: deep_merge diff --git a/test/fixtures/travis-ymls/testing-imports/merge-invalid.yml b/test/fixtures/travis-ymls/testing-imports/merge-invalid.yml new file mode 100644 index 0000000..4fc710c --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/merge-invalid.yml @@ -0,0 +1,5 @@ +language: node_js +node_js: + - "8" +import: + - source: partials/merge-invalid.yml diff --git a/test/fixtures/travis-ymls/testing-imports/merge-shallow.yml b/test/fixtures/travis-ymls/testing-imports/merge-shallow.yml new file mode 100644 index 0000000..65cb581 --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/merge-shallow.yml @@ -0,0 +1,7 @@ +language: node_js +node_js: + - "8" +import: + - source: partials/node-14.yml # default merge: deep_merge_append + - source: partials/node-12.yml + mode: merge diff --git a/test/fixtures/travis-ymls/testing-imports/partials/merge-invalid.yml b/test/fixtures/travis-ymls/testing-imports/partials/merge-invalid.yml new file mode 100644 index 0000000..ce18e6c --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/partials/merge-invalid.yml @@ -0,0 +1,7 @@ +language: node_js +node_js: + - "8" +import: + - source: partials/node-14.yml # default merge: deep_merge_append + - source: partials/node-12.yml + mode: no_such_merge_mode diff --git a/test/fixtures/travis-ymls/testing-imports/partials/node-10.yml b/test/fixtures/travis-ymls/testing-imports/partials/node-10.yml new file mode 100644 index 0000000..855b7b0 --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/partials/node-10.yml @@ -0,0 +1,3 @@ +node_js: + - "10.15" + - "10.16" diff --git a/test/fixtures/travis-ymls/testing-imports/partials/node-12.yml b/test/fixtures/travis-ymls/testing-imports/partials/node-12.yml new file mode 100644 index 0000000..c95fc11 --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/partials/node-12.yml @@ -0,0 +1,2 @@ +node_js: + - "12" diff --git a/test/travis.js b/test/travis.js index fb519ca..e6dcec4 100644 --- a/test/travis.js +++ b/test/travis.js @@ -5,6 +5,7 @@ const Nock = require('nock'); const Path = require('path'); const NodeSupport = require('..'); +const TravisMerge = require('../lib/travis/merge'); const TestContext = require('./fixtures'); @@ -157,4 +158,234 @@ describe('.travis.yml parsing', () => { }); }); + it('resolves and merges (prepend/append)', async () => { + + await fixture.setupRepoFolder({ + partials: true, + travisYml: `testing-imports/merge-deep-prepend-append.yml` + }); + + const result = await NodeSupport.detect({ path: fixture.path }); + + internals.assertCommit(result); + + expect(result).to.equal({ + name: 'test-module', + version: '0.0.0-development', + timestamp: 1580673602000, + travis: { + raw: ['12', '8', '14', '10.15', '10.16'], + resolved: { + '8': '8.17.0', + '10.15': '10.15.3', + '10.16': '10.16.3', + '12': '12.17.0', + '14': '14.3.0' + } + } + }); + }); + + it('resolves and merges (deep)', async () => { + + await fixture.setupRepoFolder({ + partials: true, + travisYml: `testing-imports/merge-deep.yml` + }); + + const result = await NodeSupport.detect({ path: fixture.path }); + + internals.assertCommit(result); + + expect(result).to.equal({ + name: 'test-module', + version: '0.0.0-development', + timestamp: 1580673602000, + travis: { + raw: ['12'], + resolved: { + '12': '12.17.0' + } + } + }); + }); + + it('resolves and merges (shallow)', async () => { + + await fixture.setupRepoFolder({ + partials: true, + travisYml: `testing-imports/merge-shallow.yml` + }); + + const result = await NodeSupport.detect({ path: fixture.path }); + + internals.assertCommit(result); + + expect(result).to.equal({ + name: 'test-module', + version: '0.0.0-development', + timestamp: 1580673602000, + travis: { + raw: ['12'], + resolved: { + '12': '12.17.0' + } + } + }); + }); + + it('throws on invalid merge mode', async () => { + + await fixture.setupRepoFolder({ + partials: true, + travisYml: `testing-imports/partials/merge-invalid.yml` + }); + + await expect(NodeSupport.detect({ path: fixture.path })).to.reject('Invalid merge mode for partials/node-12.yml in .travis.yml: no_such_merge_mode'); + }); + + it('throws on invalid merge mode (indirect)', async () => { + + await fixture.setupRepoFolder({ + partials: true, + travisYml: `testing-imports/merge-invalid.yml` + }); + + await expect(NodeSupport.detect({ path: fixture.path })).to.reject('Invalid merge mode for partials/node-12.yml in partials/merge-invalid.yml: no_such_merge_mode'); + }); +}); + +describe('Travis merging algorithms', () => { + + let left; + let right; + + beforeEach(() => { + + left = { + str1: 'left', + str2: 'left', + arr: ['left'], + obj: { + left: true, + arr: ['left'], + deep: { + left: true, + arr: ['left'] + } + }, + mix1: ['left'], + mix2: { left: true } + }; + + right = { + str1: 'right', + str3: 'right', + arr: ['right'], + obj: { + right: true, + arr: ['right'], + deep: { + right: true, + arr: ['right'] + } + }, + mix1: { right: true }, + mix2: ['right'] + }; + }); + + it('deep_merge_append', () => { + + TravisMerge.deep_merge_append(left, right); + + expect(left).to.equal({ + str1: 'right', + str2: 'left', + str3: 'right', + arr: ['left', 'right'], + obj: { + left: true, + right: true, + arr: ['left', 'right'], + deep: { + left: true, + right: true, + arr: ['left', 'right'] + } + }, + mix1: { right: true }, + mix2: ['right'] + }); + }); + + it('deep_merge_prepend', () => { + + TravisMerge.deep_merge_prepend(left, right); + + expect(left).to.equal({ + str1: 'right', + str2: 'left', + str3: 'right', + arr: ['right', 'left'], + obj: { + left: true, + right: true, + arr: ['right', 'left'], + deep: { + left: true, + right: true, + arr: ['right', 'left'] + } + }, + mix1: { right: true }, + mix2: ['right'] + }); + }); + + it('deep_merge', () => { + + TravisMerge.deep_merge(left, right); + + expect(left).to.equal({ + str1: 'right', + str2: 'left', + str3: 'right', + arr: ['right'], + obj: { + left: true, + right: true, + arr: ['right'], + deep: { + left: true, + right: true, + arr: ['right'] + } + }, + mix1: { right: true }, + mix2: ['right'] + }); + }); + + it('merge', () => { + + TravisMerge.merge(left, right); + + expect(left).to.equal({ + str1: 'right', + str2: 'left', + str3: 'right', + arr: ['right'], + obj: { + right: true, + arr: ['right'], + deep: { + right: true, + arr: ['right'] + } + }, + mix1: { right: true }, + mix2: ['right'] + }); + }); }); From cd33dc6f855ba6309eaa91b19b55c358a269789f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Mon, 15 Jun 2020 00:20:48 +0300 Subject: [PATCH 09/19] feat: cache at loader level This makes the cache global, per process, so that it can be re-used when resolving deps. --- lib/deps.js | 7 +++---- lib/loader.js | 16 ++++++++++++++++ lib/travis/imports.js | 22 +++++----------------- test/fixtures/index.js | 3 +++ test/travis.js | 5 +++++ 5 files changed, 32 insertions(+), 21 deletions(-) diff --git a/lib/deps.js b/lib/deps.js index 56113be..0754f54 100644 --- a/lib/deps.js +++ b/lib/deps.js @@ -9,10 +9,9 @@ const Tmp = require('tmp'); const Package = require('./package'); const Utils = require('./utils'); -const internals = {}; - - -internals.log = Debug('detect-node-support'); +const internals = { + log: Debug('detect-node-support') +}; internals.resolve = async ({ packageJson, lockfile }, options) => { diff --git a/lib/loader.js b/lib/loader.js index a1755d5..993ba68 100644 --- a/lib/loader.js +++ b/lib/loader.js @@ -11,6 +11,7 @@ const Wreck = require('@hapi/wreck'); const Utils = require('./utils'); const internals = { + cache: new Map(), log: Debug('detect-node-support:loader'), error: Debug('detect-node-support:error') }; @@ -95,9 +96,18 @@ internals.createRepositoryLoader = (repository) => { const url = `https://raw.githubusercontent.com/${parsedRepository.full_name}/HEAD/${filename}`; internals.log('Loading: %s', url); + if (options === undefined && internals.cache.has(url)) { + internals.log('From cache: %s', url); + return internals.cache.get(url); + } + try { const { payload } = await Wreck.get(url, options); + if (options === undefined) { + internals.cache.set(url, payload); + } + internals.log('Loaded: %s', url); return payload; } @@ -164,3 +174,9 @@ exports.create = ({ path, repository, packageName }) => { return internals.createPathLoader(path); }; + + +exports.clearCache = () => { + + internals.cache = new Map(); +}; diff --git a/lib/travis/imports.js b/lib/travis/imports.js index 3f7b6d4..453b863 100644 --- a/lib/travis/imports.js +++ b/lib/travis/imports.js @@ -1,6 +1,5 @@ 'use strict'; -const Debug = require('debug'); const Yaml = require('js-yaml'); const Loader = require('../loader'); @@ -14,9 +13,6 @@ const internals = { }; -internals.log = Debug('detect-node-support'); - - internals.normalizeImports = (travisYaml, { relativeTo }) => { return Utils.toArray(travisYaml.import) @@ -53,12 +49,7 @@ internals.normalizeImports = (travisYaml, { relativeTo }) => { }; -internals.loadSource = async (source, { loadFile, cache }) => { - - if (cache[source]) { - internals.log('Returning cached %s', source); - return cache[source]; - } +internals.loadSource = async (source, { loadFile }) => { let path = source; @@ -70,14 +61,11 @@ internals.loadSource = async (source, { loadFile, cache }) => { loadFile = loader.loadFile; } - internals.log('Loading %s', source); - const result = await loadFile(path); - cache[source] = result; - return result; + return loadFile(path); }; -exports.apply = async (yaml, { loadFile, relativeTo, cache = new Map() }) => { +exports.apply = async (yaml, { loadFile, relativeTo }) => { if (!yaml.import) { return; @@ -87,14 +75,14 @@ exports.apply = async (yaml, { loadFile, relativeTo, cache = new Map() }) => { for (const entry of imports) { - const buffer = await internals.loadSource(entry.source, { loadFile, cache }); + const buffer = await internals.loadSource(entry.source, { loadFile }); const imported = Yaml.safeLoad(buffer, { schema: Yaml.FAILSAFE_SCHEMA, json: true }); - await exports.apply(imported, { loadFile, relativeTo: entry, cache }); + await exports.apply(imported, { loadFile, relativeTo: entry }); delete imported.import; diff --git a/test/fixtures/index.js b/test/fixtures/index.js index f71f9b7..bc5cbc6 100644 --- a/test/fixtures/index.js +++ b/test/fixtures/index.js @@ -7,6 +7,7 @@ const SimpleGit = require('simple-git/promise'); const Sinon = require('sinon'); const Tmp = require('tmp'); +const Loader = require('../../lib/loader'); const Utils = require('../../lib/utils'); @@ -28,6 +29,8 @@ module.exports = class TestContext { cleanup() { + Loader.clearCache(); + Sinon.restore(); this._cleanup.forEach((cleanup) => cleanup()); diff --git a/test/travis.js b/test/travis.js index e6dcec4..2f37c3a 100644 --- a/test/travis.js +++ b/test/travis.js @@ -156,6 +156,11 @@ describe('.travis.yml parsing', () => { resolved: { '14': '14.3.0' } } }); + + // verify cache returns the same result + const result2 = await NodeSupport.detect({ path: fixture.path }); + internals.assertCommit(result2); + expect(result2).to.equal(result); }); it('resolves and merges (prepend/append)', async () => { From 60dbc0009ba6229f5c333eae04d3eba69c6abc39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Mon, 15 Jun 2020 00:32:16 +0300 Subject: [PATCH 10/19] chore: mark commitish imports as unsupported --- lib/travis/imports.js | 8 +++++++- test/fixtures/index.js | 1 + .../travis-ymls/testing-imports/commitish.yml | 3 +++ .../testing-imports/partials/commitish.yml | 3 +++ test/travis.js | 20 +++++++++++++++++++ 5 files changed, 34 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/travis-ymls/testing-imports/commitish.yml create mode 100644 test/fixtures/travis-ymls/testing-imports/partials/commitish.yml diff --git a/lib/travis/imports.js b/lib/travis/imports.js index 453b863..a845287 100644 --- a/lib/travis/imports.js +++ b/lib/travis/imports.js @@ -15,6 +15,8 @@ const internals = { internals.normalizeImports = (travisYaml, { relativeTo }) => { + const context = relativeTo ? relativeTo.source : '.travis.yml'; + return Utils.toArray(travisYaml.import) .map((entry) => { @@ -40,7 +42,11 @@ internals.normalizeImports = (travisYaml, { relativeTo }) => { } if (!internals.validMergeModes.has(entry.mode)) { - throw new Error(`Invalid merge mode for ${original} in ${relativeTo ? relativeTo.source : '.travis.yml'}: ${entry.mode}`); + throw new Error(`Invalid merge mode for ${original} in ${context}: ${entry.mode}`); + } + + if (original.includes('@')) { + throw new Error(`Importing at commitish unsupported in ${context}: ${original}`); } return entry; diff --git a/test/fixtures/index.js b/test/fixtures/index.js index bc5cbc6..0da896d 100644 --- a/test/fixtures/index.js +++ b/test/fixtures/index.js @@ -93,6 +93,7 @@ module.exports = class TestContext { if (partials) { Fs.mkdirSync(Path.join(this.path, 'partials')); const partialYmls = [ + 'commitish.yml', 'indirect-node-14.yml', 'merge-invalid.yml', 'node-10.yml', diff --git a/test/fixtures/travis-ymls/testing-imports/commitish.yml b/test/fixtures/travis-ymls/testing-imports/commitish.yml new file mode 100644 index 0000000..c53188e --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/commitish.yml @@ -0,0 +1,3 @@ +language: node_js +import: + - source: partials/commitish.yml diff --git a/test/fixtures/travis-ymls/testing-imports/partials/commitish.yml b/test/fixtures/travis-ymls/testing-imports/partials/commitish.yml new file mode 100644 index 0000000..5108b23 --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/partials/commitish.yml @@ -0,0 +1,3 @@ +language: node_js +import: + - source: partials/node-14.yml@main diff --git a/test/travis.js b/test/travis.js index 2f37c3a..e8feaef 100644 --- a/test/travis.js +++ b/test/travis.js @@ -258,6 +258,26 @@ describe('.travis.yml parsing', () => { await expect(NodeSupport.detect({ path: fixture.path })).to.reject('Invalid merge mode for partials/node-12.yml in partials/merge-invalid.yml: no_such_merge_mode'); }); + + it('throws when importing at commitish', async () => { + + await fixture.setupRepoFolder({ + partials: true, + travisYml: `testing-imports/partials/commitish.yml` + }); + + await expect(NodeSupport.detect({ path: fixture.path })).to.reject('Importing at commitish unsupported in .travis.yml: partials/node-14.yml@main'); + }); + + it('throws when importing at commitish (indirect)', async () => { + + await fixture.setupRepoFolder({ + partials: true, + travisYml: `testing-imports/commitish.yml` + }); + + await expect(NodeSupport.detect({ path: fixture.path })).to.reject('Importing at commitish unsupported in partials/commitish.yml: partials/node-14.yml@main'); + }); }); describe('Travis merging algorithms', () => { From e242cac1465e9b3484200a9cc48c6cfe2a45da9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Mon, 15 Jun 2020 00:43:19 +0300 Subject: [PATCH 11/19] feat: detect circular imports --- lib/travis/imports.js | 13 +++++++++---- test/fixtures/index.js | 1 + .../travis-ymls/testing-imports/circular.yml | 3 +++ .../testing-imports/partials/circular.yml | 3 +++ test/travis.js | 10 ++++++++++ 5 files changed, 26 insertions(+), 4 deletions(-) create mode 100644 test/fixtures/travis-ymls/testing-imports/circular.yml create mode 100644 test/fixtures/travis-ymls/testing-imports/partials/circular.yml diff --git a/lib/travis/imports.js b/lib/travis/imports.js index a845287..3ba3e7f 100644 --- a/lib/travis/imports.js +++ b/lib/travis/imports.js @@ -13,7 +13,7 @@ const internals = { }; -internals.normalizeImports = (travisYaml, { relativeTo }) => { +internals.normalizeImports = (travisYaml, { relativeTo, breadcrumb }) => { const context = relativeTo ? relativeTo.source : '.travis.yml'; @@ -49,6 +49,11 @@ internals.normalizeImports = (travisYaml, { relativeTo }) => { throw new Error(`Importing at commitish unsupported in ${context}: ${original}`); } + const alreadyImported = breadcrumb.indexOf(entry.source); + if (alreadyImported >= 0) { + throw new Error(`Circular dependency ${entry.source} requested by ${context} (already imported at ${breadcrumb[alreadyImported - 1]})`); + } + return entry; }) .filter((entry) => !entry.if); // @todo: log a warning @@ -71,13 +76,13 @@ internals.loadSource = async (source, { loadFile }) => { }; -exports.apply = async (yaml, { loadFile, relativeTo }) => { +exports.apply = async (yaml, { loadFile, relativeTo, breadcrumb = ['.travis.yml'] }) => { if (!yaml.import) { return; } - const imports = internals.normalizeImports(yaml, { relativeTo }); + const imports = internals.normalizeImports(yaml, { relativeTo, breadcrumb }); for (const entry of imports) { @@ -88,7 +93,7 @@ exports.apply = async (yaml, { loadFile, relativeTo }) => { json: true }); - await exports.apply(imported, { loadFile, relativeTo: entry }); + await exports.apply(imported, { loadFile, relativeTo: entry, breadcrumb: [...breadcrumb, entry.source] }); delete imported.import; diff --git a/test/fixtures/index.js b/test/fixtures/index.js index 0da896d..b8acdd4 100644 --- a/test/fixtures/index.js +++ b/test/fixtures/index.js @@ -93,6 +93,7 @@ module.exports = class TestContext { if (partials) { Fs.mkdirSync(Path.join(this.path, 'partials')); const partialYmls = [ + 'circular.yml', 'commitish.yml', 'indirect-node-14.yml', 'merge-invalid.yml', diff --git a/test/fixtures/travis-ymls/testing-imports/circular.yml b/test/fixtures/travis-ymls/testing-imports/circular.yml new file mode 100644 index 0000000..10817ad --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/circular.yml @@ -0,0 +1,3 @@ +language: node_js +import: + - source: partials/circular.yml diff --git a/test/fixtures/travis-ymls/testing-imports/partials/circular.yml b/test/fixtures/travis-ymls/testing-imports/partials/circular.yml new file mode 100644 index 0000000..10817ad --- /dev/null +++ b/test/fixtures/travis-ymls/testing-imports/partials/circular.yml @@ -0,0 +1,3 @@ +language: node_js +import: + - source: partials/circular.yml diff --git a/test/travis.js b/test/travis.js index e8feaef..36f01dc 100644 --- a/test/travis.js +++ b/test/travis.js @@ -278,6 +278,16 @@ describe('.travis.yml parsing', () => { await expect(NodeSupport.detect({ path: fixture.path })).to.reject('Importing at commitish unsupported in partials/commitish.yml: partials/node-14.yml@main'); }); + + it('throws when importing a circular dependency', async () => { + + await fixture.setupRepoFolder({ + partials: true, + travisYml: `testing-imports/circular.yml` + }); + + await expect(NodeSupport.detect({ path: fixture.path })).to.reject('Circular dependency partials/circular.yml requested by partials/circular.yml (already imported at .travis.yml)'); + }); }); describe('Travis merging algorithms', () => { From c18f2b8f47bb269be1fae4f2b35c123daa1f5b70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Mon, 15 Jun 2020 11:39:00 +0300 Subject: [PATCH 12/19] refactor: move loader.js into loader/index.js --- lib/{loader.js => loader/index.js} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename lib/{loader.js => loader/index.js} (98%) diff --git a/lib/loader.js b/lib/loader/index.js similarity index 98% rename from lib/loader.js rename to lib/loader/index.js index 993ba68..2faec57 100644 --- a/lib/loader.js +++ b/lib/loader/index.js @@ -3,12 +3,12 @@ const Debug = require('debug'); const Fs = require('fs'); const GitUrlParse = require('git-url-parse'); -const Package = require('../package.json'); +const Package = require('../../package.json'); const Pacote = require('pacote'); const Path = require('path'); const Wreck = require('@hapi/wreck'); -const Utils = require('./utils'); +const Utils = require('../utils'); const internals = { cache: new Map(), From 2d965937eef24eef6e5d58fe32baa1f04ec01e24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Mon, 15 Jun 2020 11:52:48 +0300 Subject: [PATCH 13/19] refactor: split up loader --- lib/loader/index.js | 175 ++------------------------------------- lib/loader/npm.js | 60 ++++++++++++++ lib/loader/path.js | 40 +++++++++ lib/loader/repository.js | 79 ++++++++++++++++++ test/fixtures/index.js | 4 +- 5 files changed, 187 insertions(+), 171 deletions(-) create mode 100644 lib/loader/npm.js create mode 100644 lib/loader/path.js create mode 100644 lib/loader/repository.js diff --git a/lib/loader/index.js b/lib/loader/index.js index 2faec57..710ac37 100644 --- a/lib/loader/index.js +++ b/lib/loader/index.js @@ -1,182 +1,19 @@ 'use strict'; -const Debug = require('debug'); -const Fs = require('fs'); -const GitUrlParse = require('git-url-parse'); -const Package = require('../../package.json'); -const Pacote = require('pacote'); -const Path = require('path'); -const Wreck = require('@hapi/wreck'); - -const Utils = require('../utils'); - -const internals = { - cache: new Map(), - log: Debug('detect-node-support:loader'), - error: Debug('detect-node-support:error') -}; - - -internals.parseRepository = (packument) => { - - if (typeof packument.repository === 'string') { - return packument.repository; - } - - if (!packument.repository || !packument.repository.url) { - throw new Error(`Unable to determine the git repository for ${packument.name}`); - } - - return packument.repository.url; -}; - - -internals.createPackageLoader = async (packageName) => { - - try { - const packument = await Pacote.packument(packageName + '@latest', { - 'fullMetadata': true, - 'user-agent': `${Package.name}@${Package.version}, see ${Package.homepage}` - }); - - const repository = internals.parseRepository(packument); - - const repositoryLoader = internals.createRepositoryLoader(repository); - - return { - ...repositoryLoader, - loadFile: async (filename, options) => { - - const result = await repositoryLoader.loadFile(filename, options); - - if (filename === 'package.json' && result.name !== packageName) { - throw new Error(`${repository} does not contain ${packageName}. Monorepo not supported: https://github.com/pkgjs/detect-node-support/issues/6`); - } - - return result; - } - }; - } - catch (err) { - - if (err.statusCode === 404) { - throw new Error(`Package ${packageName} does not exist`); - } - - throw err; - - } -}; - - -internals.createRepositoryLoader = (repository) => { - - if (repository.split('/').length === 2) { - repository = `https://github.com/${repository}`; - } - - const parsedRepository = GitUrlParse(repository); - - return { - getCommit: async () => { - - const simpleGit = Utils.simpleGit(); - const httpRepository = GitUrlParse.stringify(parsedRepository, 'http'); - const result = await simpleGit.listRemote([httpRepository, 'HEAD']); - const [head] = result.split(/\s+/); - - return head; - }, - loadFile: async (filename, options) => { - - if (parsedRepository.source !== 'github.com') { - throw new Error('Only github.com paths supported, feel free to PR at https://github.com/pkgjs/detect-node-support'); - } - - const url = `https://raw.githubusercontent.com/${parsedRepository.full_name}/HEAD/${filename}`; - internals.log('Loading: %s', url); - - if (options === undefined && internals.cache.has(url)) { - internals.log('From cache: %s', url); - return internals.cache.get(url); - } - - try { - const { payload } = await Wreck.get(url, options); - - if (options === undefined) { - internals.cache.set(url, payload); - } - - internals.log('Loaded: %s', url); - return payload; - } - catch (err) { - - if (err.data && err.data.res.statusCode === 404) { - internals.log('Not found: %s', url); - const error = new Error(`${repository} does not contain a ${filename}`); - error.code = 'ENOENT'; - throw error; - } - - internals.error('Failed to load: %s', url); - throw err; - } - } - }; -}; - - -internals.createPathLoader = async (path) => { - - const simpleGit = Utils.simpleGit(path); - const isRepo = await simpleGit.checkIsRepo(); - - if (!isRepo) { - throw new Error(`${path} is not a git repository`); - } - - if (!Fs.existsSync(Path.join(path, 'package.json'))) { - throw new Error(`${path} does not contain a package.json`); - } - - return { - getCommit: () => { - - return simpleGit.revparse(['HEAD']); - }, - loadFile: (filename, options = {}) => { - - const fullPath = Path.join(path, filename); - - const buffer = Fs.readFileSync(fullPath); - - if (options.json) { - return JSON.parse(buffer.toString()); - } - - return buffer; - } - }; -}; +const NpmLoader = require('./npm'); +const PathLoader = require('./path'); +const RepositoryLoader = require('./repository'); exports.create = ({ path, repository, packageName }) => { if (repository) { - return internals.createRepositoryLoader(repository); + return RepositoryLoader.create(repository); } if (packageName) { - return internals.createPackageLoader(packageName); + return NpmLoader.create(packageName); } - return internals.createPathLoader(path); -}; - - -exports.clearCache = () => { - - internals.cache = new Map(); + return PathLoader.create(path); }; diff --git a/lib/loader/npm.js b/lib/loader/npm.js new file mode 100644 index 0000000..fefcf1c --- /dev/null +++ b/lib/loader/npm.js @@ -0,0 +1,60 @@ +'use strict'; + +const Package = require('../../package.json'); +const Pacote = require('pacote'); + +const RepositoryLoader = require('./repository'); + +const internals = {}; + + +internals.parseRepository = (packument) => { + + if (typeof packument.repository === 'string') { + return packument.repository; + } + + if (!packument.repository || !packument.repository.url) { + throw new Error(`Unable to determine the git repository for ${packument.name}`); + } + + return packument.repository.url; +}; + + +exports.create = async (packageName) => { + + try { + const packument = await Pacote.packument(packageName + '@latest', { + 'fullMetadata': true, + 'user-agent': `${Package.name}@${Package.version}, see ${Package.homepage}` + }); + + const repository = internals.parseRepository(packument); + + const repositoryLoader = RepositoryLoader.create(repository); + + return { + ...repositoryLoader, + loadFile: async (filename, options) => { + + const result = await repositoryLoader.loadFile(filename, options); + + if (filename === 'package.json' && result.name !== packageName) { + throw new Error(`${repository} does not contain ${packageName}. Monorepo not supported: https://github.com/pkgjs/detect-node-support/issues/6`); + } + + return result; + } + }; + } + catch (err) { + + if (err.statusCode === 404) { + throw new Error(`Package ${packageName} does not exist`); + } + + throw err; + + } +}; diff --git a/lib/loader/path.js b/lib/loader/path.js new file mode 100644 index 0000000..4e52f06 --- /dev/null +++ b/lib/loader/path.js @@ -0,0 +1,40 @@ +'use strict'; + +const Fs = require('fs'); +const Path = require('path'); + +const Utils = require('../utils'); + + +exports.create = async (path) => { + + const simpleGit = Utils.simpleGit(path); + const isRepo = await simpleGit.checkIsRepo(); + + if (!isRepo) { + throw new Error(`${path} is not a git repository`); + } + + if (!Fs.existsSync(Path.join(path, 'package.json'))) { + throw new Error(`${path} does not contain a package.json`); + } + + return { + getCommit: () => { + + return simpleGit.revparse(['HEAD']); + }, + loadFile: (filename, options = {}) => { + + const fullPath = Path.join(path, filename); + + const buffer = Fs.readFileSync(fullPath); + + if (options.json) { + return JSON.parse(buffer.toString()); + } + + return buffer; + } + }; +}; diff --git a/lib/loader/repository.js b/lib/loader/repository.js new file mode 100644 index 0000000..fd29dad --- /dev/null +++ b/lib/loader/repository.js @@ -0,0 +1,79 @@ +'use strict'; + +const Debug = require('debug'); +const GitUrlParse = require('git-url-parse'); +const Wreck = require('@hapi/wreck'); + +const Utils = require('../utils'); + + +const internals = { + cache: new Map(), + log: Debug('detect-node-support:loader'), + error: Debug('detect-node-support:error') +}; + + +exports.create = (repository) => { + + if (repository.split('/').length === 2) { + repository = `https://github.com/${repository}`; + } + + const parsedRepository = GitUrlParse(repository); + + return { + getCommit: async () => { + + const simpleGit = Utils.simpleGit(); + const httpRepository = GitUrlParse.stringify(parsedRepository, 'http'); + const result = await simpleGit.listRemote([httpRepository, 'HEAD']); + const [head] = result.split(/\s+/); + + return head; + }, + loadFile: async (filename, options) => { + + if (parsedRepository.source !== 'github.com') { + throw new Error('Only github.com paths supported, feel free to PR at https://github.com/pkgjs/detect-node-support'); + } + + const url = `https://raw.githubusercontent.com/${parsedRepository.full_name}/HEAD/${filename}`; + internals.log('Loading: %s', url); + + if (options === undefined && internals.cache.has(url)) { + internals.log('From cache: %s', url); + return internals.cache.get(url); + } + + try { + const { payload } = await Wreck.get(url, options); + + if (options === undefined) { + internals.cache.set(url, payload); + } + + internals.log('Loaded: %s', url); + return payload; + } + catch (err) { + + if (err.data && err.data.res.statusCode === 404) { + internals.log('Not found: %s', url); + const error = new Error(`${repository} does not contain a ${filename}`); + error.code = 'ENOENT'; + throw error; + } + + internals.error('Failed to load: %s', url); + throw err; + } + } + }; +}; + + +exports.clearCache = () => { + + internals.cache = new Map(); +}; diff --git a/test/fixtures/index.js b/test/fixtures/index.js index b8acdd4..b64d2ce 100644 --- a/test/fixtures/index.js +++ b/test/fixtures/index.js @@ -7,7 +7,7 @@ const SimpleGit = require('simple-git/promise'); const Sinon = require('sinon'); const Tmp = require('tmp'); -const Loader = require('../../lib/loader'); +const RepositoryLoader = require('../../lib/loader/repository'); const Utils = require('../../lib/utils'); @@ -29,7 +29,7 @@ module.exports = class TestContext { cleanup() { - Loader.clearCache(); + RepositoryLoader.clearCache(); Sinon.restore(); From e8269647f47c3655b5bde4cbb007c2e442ee67d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Mon, 15 Jun 2020 11:53:27 +0300 Subject: [PATCH 14/19] refactor: extract userAgent constants --- lib/constants.js | 6 ++++++ lib/loader/npm.js | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) create mode 100644 lib/constants.js diff --git a/lib/constants.js b/lib/constants.js new file mode 100644 index 0000000..ad53965 --- /dev/null +++ b/lib/constants.js @@ -0,0 +1,6 @@ +'use strict'; + +const Package = require('../package.json'); + + +exports.userAgent = `${Package.name}/${Package.version} (${Package.homepage})`; diff --git a/lib/loader/npm.js b/lib/loader/npm.js index fefcf1c..56b2e86 100644 --- a/lib/loader/npm.js +++ b/lib/loader/npm.js @@ -1,8 +1,8 @@ 'use strict'; -const Package = require('../../package.json'); const Pacote = require('pacote'); +const Constants = require('../constants'); const RepositoryLoader = require('./repository'); const internals = {}; @@ -27,7 +27,7 @@ exports.create = async (packageName) => { try { const packument = await Pacote.packument(packageName + '@latest', { 'fullMetadata': true, - 'user-agent': `${Package.name}@${Package.version}, see ${Package.homepage}` + 'user-agent': Constants.userAgent }); const repository = internals.parseRepository(packument); From 1dc4767b47799b51521b5fe8a491d833597f5b05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Sat, 20 Jun 2020 13:13:36 +0300 Subject: [PATCH 15/19] refactor: extract logger --- bin/detect-node-support | 5 ++--- lib/deps.js | 15 +++++++++------ lib/loader/repository.js | 16 +++++++--------- lib/logger.js | 39 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 18 deletions(-) create mode 100644 lib/logger.js diff --git a/bin/detect-node-support b/bin/detect-node-support index bf39426..9b79131 100755 --- a/bin/detect-node-support +++ b/bin/detect-node-support @@ -13,9 +13,6 @@ const NodeSupport = require('..'); const internals = {}; -internals.log = Debug('detect-node-support'); - - internals.help = () => { return ` @@ -34,6 +31,8 @@ Options: exports.main = async ({ _: [what], deps, deep, dev, json }) => { + Debug.enable('detect-node-support:warn:*,detect-node-support:error:*'); + if (!what) { console.log(internals.help()); return; diff --git a/lib/deps.js b/lib/deps.js index 0754f54..41245f9 100644 --- a/lib/deps.js +++ b/lib/deps.js @@ -1,17 +1,15 @@ 'use strict'; -const Debug = require('debug'); const { Arborist } = require('@npmcli/arborist'); const Fs = require('fs'); const Path = require('path'); const Tmp = require('tmp'); +const Logger = require('./logger'); const Package = require('./package'); const Utils = require('./utils'); -const internals = { - log: Debug('detect-node-support') -}; +const internals = {}; internals.resolve = async ({ packageJson, lockfile }, options) => { @@ -72,7 +70,12 @@ internals.tryLoad = async (loadFile, filename) => { exports.detect = async ({ packageJson, loadFile }, options) => { const lockfile = (await internals.tryLoad(loadFile, 'package-lock.json')) || (await internals.tryLoad(loadFile, 'npm-shrinkwrap.json')); - internals.log(lockfile ? 'Lock file present' : 'Lock file missing - things will be a bit slower'); + if (lockfile) { + Logger.log(['deps'], 'Lock file present'); + } + else { + Logger.warn(['deps'], 'Lock file missing - things will be a bit slower'); + } const versions = await internals.resolve({ packageJson, lockfile }, options); @@ -86,7 +89,7 @@ exports.detect = async ({ packageJson, loadFile }, options) => { for (let i = 0; i < n; ++i) { const packageName = packages[i]; - internals.log(`Resolving dependency ${i + 1} of ${n}: ${packageName}`); + Logger.log(['deps'], `Resolving dependency ${i + 1} of ${n}: ${packageName}`); try { const { result } = await Package.detect({ packageName }); diff --git a/lib/loader/repository.js b/lib/loader/repository.js index fd29dad..a1aca16 100644 --- a/lib/loader/repository.js +++ b/lib/loader/repository.js @@ -1,16 +1,14 @@ 'use strict'; -const Debug = require('debug'); const GitUrlParse = require('git-url-parse'); const Wreck = require('@hapi/wreck'); +const Logger = require('../logger'); const Utils = require('../utils'); const internals = { - cache: new Map(), - log: Debug('detect-node-support:loader'), - error: Debug('detect-node-support:error') + cache: new Map() }; @@ -39,10 +37,10 @@ exports.create = (repository) => { } const url = `https://raw.githubusercontent.com/${parsedRepository.full_name}/HEAD/${filename}`; - internals.log('Loading: %s', url); + Logger.log(['loader'], 'Loading: %s', url); if (options === undefined && internals.cache.has(url)) { - internals.log('From cache: %s', url); + Logger.log(['loader'], 'From cache: %s', url); return internals.cache.get(url); } @@ -53,19 +51,19 @@ exports.create = (repository) => { internals.cache.set(url, payload); } - internals.log('Loaded: %s', url); + Logger.log(['loader'], 'Loaded: %s', url); return payload; } catch (err) { if (err.data && err.data.res.statusCode === 404) { - internals.log('Not found: %s', url); + Logger.log(['loader'], 'Not found: %s', url); const error = new Error(`${repository} does not contain a ${filename}`); error.code = 'ENOENT'; throw error; } - internals.error('Failed to load: %s', url); + Logger.error(['loader'], 'Failed to load: %s', url); throw err; } } diff --git a/lib/logger.js b/lib/logger.js new file mode 100644 index 0000000..b7b7c37 --- /dev/null +++ b/lib/logger.js @@ -0,0 +1,39 @@ +'use strict'; + +const Debug = require('debug'); + + +const internals = { + loggers: {} +}; + + +internals.getLogger = (tags) => { + + const suffix = tags.join(':'); + + if (!internals.loggers[suffix]) { + internals.loggers[suffix] = Debug(`detect-node-support:${suffix}`); + } + + return internals.loggers[suffix]; +}; + + +exports.log = (tags, ...args) => { + + const logger = internals.getLogger(tags); + logger(...args); +}; + + +exports.warn = (tags, ...args) => { + + exports.log(['warn', ...tags], ...args); +}; + + +exports.error = (tags, ...args) => { + + exports.log(['error', ...tags], ...args); +}; From e7fbc6c0dc5d2baa85d771bebb44bf77f97a365e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Mon, 15 Jun 2020 13:53:34 +0300 Subject: [PATCH 16/19] refactor: use octokit for fetching from github --- lib/loader/octokit-wrapper.js | 14 ++ lib/loader/repository.js | 47 ++++--- lib/package.js | 2 +- package.json | 2 +- test/index.js | 234 +++++++++++++++++++++------------- test/travis.js | 14 +- 6 files changed, 202 insertions(+), 111 deletions(-) create mode 100644 lib/loader/octokit-wrapper.js diff --git a/lib/loader/octokit-wrapper.js b/lib/loader/octokit-wrapper.js new file mode 100644 index 0000000..a212b22 --- /dev/null +++ b/lib/loader/octokit-wrapper.js @@ -0,0 +1,14 @@ +'use strict'; + +const { Octokit } = require('@octokit/rest'); + +exports.create = () => { + + const octokit = new Octokit(); + + // @todo: onRateLimit + // @todo: auth + // @todo: user agent + + return octokit; +}; diff --git a/lib/loader/repository.js b/lib/loader/repository.js index a1aca16..8628581 100644 --- a/lib/loader/repository.js +++ b/lib/loader/repository.js @@ -1,9 +1,9 @@ 'use strict'; const GitUrlParse = require('git-url-parse'); -const Wreck = require('@hapi/wreck'); const Logger = require('../logger'); +const OctokitWrapper = require('./octokit-wrapper'); const Utils = require('../utils'); @@ -30,40 +30,55 @@ exports.create = (repository) => { return head; }, - loadFile: async (filename, options) => { + loadFile: async (filename, options = {}) => { if (parsedRepository.source !== 'github.com') { throw new Error('Only github.com paths supported, feel free to PR at https://github.com/pkgjs/detect-node-support'); } - const url = `https://raw.githubusercontent.com/${parsedRepository.full_name}/HEAD/${filename}`; - Logger.log(['loader'], 'Loading: %s', url); + const resource = `${parsedRepository.full_name}:${filename}@HEAD`; + Logger.log(['loader'], 'Loading: %s', resource); - if (options === undefined && internals.cache.has(url)) { - Logger.log(['loader'], 'From cache: %s', url); - return internals.cache.get(url); - } + const octokit = OctokitWrapper.create(); try { - const { payload } = await Wreck.get(url, options); - if (options === undefined) { - internals.cache.set(url, payload); + let result; + if (internals.cache.has(resource)) { + Logger.log(['loader'], 'From cache: %s', resource); + result = internals.cache.get(resource); + } + else { + result = await octokit.repos.getContent({ + owner: parsedRepository.owner, + repo: parsedRepository.name, + path: filename + }); + } + + internals.cache.set(resource, result); + + Logger.log(['loader'], 'Loaded: %s', resource); + + const content = Buffer.from(result.data.content, 'base64'); + + if (options.json) { + // @todo: cache parsed JSON, parse YAML + return JSON.parse(content.toString()); } - Logger.log(['loader'], 'Loaded: %s', url); - return payload; + return content; } catch (err) { - if (err.data && err.data.res.statusCode === 404) { - Logger.log(['loader'], 'Not found: %s', url); + if (err.status === 404) { + Logger.log(['loader'], 'Not found: %s', resource); const error = new Error(`${repository} does not contain a ${filename}`); error.code = 'ENOENT'; throw error; } - Logger.error(['loader'], 'Failed to load: %s', url); + Logger.error(['loader'], 'Failed to load: %s', resource); throw err; } } diff --git a/lib/package.js b/lib/package.js index 9c88080..8077699 100644 --- a/lib/package.js +++ b/lib/package.js @@ -45,7 +45,7 @@ exports.detect = async (what) => { const { loadFile, getCommit } = await Loader.create({ path, repository, packageName }); - const packageJson = await loadFile('package.json', { json: 'force' }); + const packageJson = await loadFile('package.json', { json: true }); const meta = { packageJson, diff --git a/package.json b/package.json index 0b38781..1d462af 100644 --- a/package.json +++ b/package.json @@ -35,8 +35,8 @@ "sinon": "^9.0.0" }, "dependencies": { - "@hapi/wreck": "^17.0.0", "@npmcli/arborist": "0.0.0-pre.21", + "@octokit/rest": "^18.0.0", "@pkgjs/nv": "0.0.3", "debug": "^4.1.1", "git-url-parse": "^11.1.2", diff --git a/test/index.js b/test/index.js index ced0d70..1408ef6 100644 --- a/test/index.js +++ b/test/index.js @@ -4,10 +4,10 @@ const Fs = require('fs'); const Nock = require('nock'); const Path = require('path'); const Sinon = require('sinon'); -const Wreck = require('@hapi/wreck'); const NodeSupport = require('..'); +const OctokitWrapper = require('../lib/loader/octokit-wrapper'); const TestContext = require('./fixtures'); @@ -426,11 +426,15 @@ describe('detect-node-support', () => { fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); - Nock('https://raw.githubusercontent.com') - .get('/pkgjs/detect-node-support/HEAD/package.json') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', 'package.json'))) - .get('/pkgjs/detect-node-support/HEAD/.travis.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml'))); + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', 'package.json')).toString('base64') + }) + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml')).toString('base64') + }); const result = await NodeSupport.detect({ repository: 'git+https://github.com/pkgjs/detect-node-support.git' }); @@ -459,11 +463,15 @@ describe('detect-node-support', () => { fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); - Nock('https://raw.githubusercontent.com') - .get('/pkgjs/detect-node-support/HEAD/package.json') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', 'package.json'))) - .get('/pkgjs/detect-node-support/HEAD/.travis.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml'))); + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', 'package.json')).toString('base64') + }) + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml')).toString('base64') + }); const result = await NodeSupport.detect({ repository: 'pkgjs/detect-node-support' }); @@ -492,10 +500,12 @@ describe('detect-node-support', () => { fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); - Nock('https://raw.githubusercontent.com') - .get('/pkgjs/detect-node-support/HEAD/package.json') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', 'package.json'))) - .get('/pkgjs/detect-node-support/HEAD/.travis.yml') + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', 'package.json')).toString('base64') + }) + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') .reply(404); const result = await NodeSupport.detect({ repository: 'git+https://github.com/pkgjs/detect-node-support.git' }); @@ -517,14 +527,16 @@ describe('detect-node-support', () => { fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); - Nock('https://raw.githubusercontent.com') - .get('/pkgjs/detect-node-support/HEAD/package.json') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', 'package.json'))) - .get('/pkgjs/detect-node-support/HEAD/.travis.yml') + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', 'package.json')).toString('base64') + }) + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') .reply(500); - await expect(NodeSupport.detect({ repository: 'git+https://github.com/pkgjs/detect-node-support.git' })) - .to.reject('Response Error: 500 null'); // the null is a Nock/Wreck implementation detail + const err = await expect(NodeSupport.detect({ repository: 'git+https://github.com/pkgjs/detect-node-support.git' })).to.reject(); + expect(err.name).to.equal('HttpError'); }); it('throws when repository does not have a package.json', async () => { @@ -532,11 +544,13 @@ describe('detect-node-support', () => { fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); - Nock('https://raw.githubusercontent.com') - .get('/pkgjs/detect-node-support/HEAD/package.json') + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') .reply(404) - .get('/pkgjs/detect-node-support/HEAD/.travis.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml'))); + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml')).toString('base64') + }); await expect(NodeSupport.detect({ repository: 'git+https://github.com/pkgjs/detect-node-support.git' })) .to.reject(`git+https://github.com/pkgjs/detect-node-support.git does not contain a package.json`); @@ -544,21 +558,23 @@ describe('detect-node-support', () => { it('rethrows server errors', async () => { - Nock('https://raw.githubusercontent.com') - .get('/pkgjs/detect-node-support/HEAD/package.json') + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') .reply(500) - .get('/pkgjs/detect-node-support/HEAD/.travis.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml'))); + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml')).toString('base64') + }); - await expect(NodeSupport.detect({ repository: 'git+https://github.com/pkgjs/detect-node-support.git' })) - .to.reject(/Response Error/); + const err = await expect(NodeSupport.detect({ repository: 'git+https://github.com/pkgjs/detect-node-support.git' })).to.reject(); + expect(err.name).to.equal('HttpError'); }); it('rethrows generic errors', async () => { const err = new Error('Something went wrong'); - Sinon.stub(Wreck, 'get').throws(err); + Sinon.stub(OctokitWrapper, 'create').throws(err); await expect(NodeSupport.detect({ repository: 'git+https://github.com/pkgjs/detect-node-support.git' })) .to.reject('Something went wrong'); @@ -578,11 +594,15 @@ describe('detect-node-support', () => { fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); - Nock('https://raw.githubusercontent.com') - .get('/pkgjs/detect-node-support/HEAD/package.json') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', 'package.json'))) - .get('/pkgjs/detect-node-support/HEAD/.travis.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml'))); + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', 'package.json')).toString('base64') + }) + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml')).toString('base64') + }); Nock('https://registry.npmjs.org') .get('/detect-node-support') @@ -615,10 +635,12 @@ describe('detect-node-support', () => { fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); - Nock('https://raw.githubusercontent.com') - .get('/pkgjs/detect-node-support/HEAD/package.json') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', 'package.json'))) - .get('/pkgjs/detect-node-support/HEAD/.travis.yml') + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', 'package.json')).toString('base64') + }) + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') .reply(404); Nock('https://registry.npmjs.org') @@ -667,7 +689,7 @@ describe('detect-node-support', () => { const err = new Error('Something went wrong'); - Sinon.stub(Wreck, 'get').throws(err); + Sinon.stub(OctokitWrapper, 'create').throws(err); await expect(NodeSupport.detect({ packageName: 'detect-node-support' })) .to.reject('Something went wrong'); @@ -698,11 +720,15 @@ describe('detect-node-support', () => { fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); - Nock('https://raw.githubusercontent.com') - .get('/pkgjs/detect-node-support/HEAD/package.json') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', 'package.json'))) - .get('/pkgjs/detect-node-support/HEAD/.travis.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml'))); + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', 'package.json')).toString('base64') + }) + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml')).toString('base64') + }); Nock('https://registry.npmjs.org') .get('/detect-node-support') @@ -738,11 +764,15 @@ describe('detect-node-support', () => { fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); - Nock('https://raw.githubusercontent.com') - .get('/pkgjs/detect-node-support/HEAD/package.json') - .reply(200, JSON.stringify({ name: 'something-else' })) - .get('/pkgjs/detect-node-support/HEAD/.travis.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml'))); + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') + .reply(200, { + content: Buffer.from(JSON.stringify({ name: 'something-else' })).toString('base64') + }) + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml')).toString('base64') + }); Nock('https://registry.npmjs.org') .get('/detect-node-support') @@ -784,11 +814,15 @@ describe('detect-node-support', () => { fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); - Nock('https://raw.githubusercontent.com') - .get('/pkgjs/detect-node-support/HEAD/package.json') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', 'package.json'))) - .get('/pkgjs/detect-node-support/HEAD/.travis.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml'))); + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', 'package.json')).toString('base64') + }) + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml')).toString('base64') + }); const result = await NodeSupport.detect('git+https://github.com/pkgjs/detect-node-support.git'); @@ -817,11 +851,15 @@ describe('detect-node-support', () => { fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); - Nock('https://raw.githubusercontent.com') - .get('/pkgjs/detect-node-support/HEAD/package.json') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', 'package.json'))) - .get('/pkgjs/detect-node-support/HEAD/.travis.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml'))); + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', 'package.json')).toString('base64') + }) + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml')).toString('base64') + }); const result = await NodeSupport.detect('pkgjs/detect-node-support'); @@ -850,11 +888,15 @@ describe('detect-node-support', () => { fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); - Nock('https://raw.githubusercontent.com') - .get('/pkgjs/detect-node-support/HEAD/package.json') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', 'package.json'))) - .get('/pkgjs/detect-node-support/HEAD/.travis.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml'))); + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', 'package.json')).toString('base64') + }) + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml')).toString('base64') + }); Nock('https://registry.npmjs.org') .get('/detect-node-support') @@ -887,11 +929,15 @@ describe('detect-node-support', () => { fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); - Nock('https://raw.githubusercontent.com') - .get('/hapijs/hapi/HEAD/package.json') - .reply(200, Fs.readFileSync(Path.join(__dirname, 'fixtures', 'hapi-package.json'))) - .get('/hapijs/hapi/HEAD/.travis.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml'))); + Nock('https://api.github.com') + .get('/repos/hapijs/hapi/contents/package.json') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, 'fixtures', 'hapi-package.json')).toString('base64') + }) + .get('/repos/hapijs/hapi/contents/.travis.yml') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml')).toString('base64') + }); Nock('https://registry.npmjs.org') .get('/@hapi%2fhapi') @@ -939,24 +985,36 @@ describe('detect-node-support', () => { .get('/rimraf') .reply(200, Fs.readFileSync(Path.join(__dirname, 'fixtures', 'packuments', 'rimraf.json'))); - Nock('https://raw.githubusercontent.com') - .get('/watson/is-ci/HEAD/package.json') - .reply(200, JSON.stringify({ name: 'is-ci', version: '2.0.0' })) - .get('/watson/is-ci/HEAD/.travis.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, 'fixtures', 'travis-ymls', 'testing-single-version.yml'))) - .get('/watson/ci-info/HEAD/package.json') - .reply(200, JSON.stringify({ name: 'ci-info', version: '2.0.0' })) - .get('/watson/ci-info/HEAD/.travis.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, 'fixtures', 'travis-ymls', 'testing-single-version.yml'))) - .get('/visionmedia/debug/HEAD/package.json') - .reply(200, JSON.stringify({ name: 'debug', version: '4.1.1' })) - .get('/visionmedia/debug/HEAD/.travis.yml') + Nock('https://api.github.com') + .get('/repos/watson/is-ci/contents/package.json') + .reply(200, { + content: Buffer.from(JSON.stringify({ name: 'is-ci', version: '2.0.0' })).toString('base64') + }) + .get('/repos/watson/is-ci/contents/.travis.yml') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, 'fixtures', 'travis-ymls', 'testing-single-version.yml')).toString('base64') + }) + .get('/repos/watson/ci-info/contents/package.json') + .reply(200, { + content: Buffer.from(JSON.stringify({ name: 'ci-info', version: '2.0.0' })).toString('base64') + }) + .get('/repos/watson/ci-info/contents/.travis.yml') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, 'fixtures', 'travis-ymls', 'testing-single-version.yml')).toString('base64') + }) + .get('/repos/visionmedia/debug/contents/package.json') + .reply(200, { + content: Buffer.from(JSON.stringify({ name: 'debug', version: '4.1.1' })).toString('base64') + }) + .get('/repos/visionmedia/debug/contents/.travis.yml') .reply(404) - .get('/zeit/ms/HEAD/package.json') - .reply(200, JSON.stringify({ name: 'ms', version: '2.1.2' })) - .get('/zeit/ms/HEAD/.travis.yml') + .get('/repos/zeit/ms/contents/package.json') + .reply(200, { + content: Buffer.from(JSON.stringify({ name: 'ms', version: '2.1.2' })).toString('base64') + }) + .get('/repos/zeit/ms/contents/.travis.yml') .reply(404) - .get('/isaacs/rimraf/HEAD/package.json') + .get('/repos/isaacs/rimraf/contents/package.json') .reply(404); }); diff --git a/test/travis.js b/test/travis.js index 36f01dc..1f4b2ab 100644 --- a/test/travis.js +++ b/test/travis.js @@ -132,11 +132,15 @@ describe('.travis.yml parsing', () => { it('resolves from another repo', async () => { - Nock('https://raw.githubusercontent.com') - .get('/pkgjs/detect-node-support/HEAD/test/fixtures/travis-ymls/testing-imports/partials/indirect-node-14.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, 'fixtures', 'travis-ymls', 'testing-imports', 'partials', 'indirect-node-14.yml'))) - .get('/pkgjs/detect-node-support/HEAD/test/fixtures/travis-ymls/testing-imports/partials/node-14.yml') - .reply(200, Fs.readFileSync(Path.join(__dirname, 'fixtures', 'travis-ymls', 'testing-imports', 'partials', 'node-14.yml'))); + Nock('https://api.github.com') + .get(`/repos/pkgjs/detect-node-support/contents/${encodeURIComponent('test/fixtures/travis-ymls/testing-imports/partials/indirect-node-14.yml')}`) + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, 'fixtures', 'travis-ymls', 'testing-imports', 'partials', 'indirect-node-14.yml')).toString('base64') + }) + .get(`/repos/pkgjs/detect-node-support/contents/${encodeURIComponent('test/fixtures/travis-ymls/testing-imports/partials/node-14.yml')}`) + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, 'fixtures', 'travis-ymls', 'testing-imports', 'partials', 'node-14.yml')).toString('base64') + }); await fixture.setupRepoFolder({ partials: true, From 759c508e8141940edb8f4febf7a8ecf1cf015138 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Mon, 15 Jun 2020 16:57:30 +0300 Subject: [PATCH 17/19] fix: set user agent for octokit requests --- lib/loader/octokit-wrapper.js | 7 +++++-- test/index.js | 6 +++++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/loader/octokit-wrapper.js b/lib/loader/octokit-wrapper.js index a212b22..9469f45 100644 --- a/lib/loader/octokit-wrapper.js +++ b/lib/loader/octokit-wrapper.js @@ -2,13 +2,16 @@ const { Octokit } = require('@octokit/rest'); +const Constants = require('../constants'); + exports.create = () => { - const octokit = new Octokit(); + const octokit = new Octokit({ + userAgent: Constants.userAgent + }); // @todo: onRateLimit // @todo: auth - // @todo: user agent return octokit; }; diff --git a/test/index.js b/test/index.js index 1408ef6..7c1c3d6 100644 --- a/test/index.js +++ b/test/index.js @@ -594,7 +594,11 @@ describe('detect-node-support', () => { fixture.stubs.listRemote .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); - Nock('https://api.github.com') + Nock('https://api.github.com', { + reqheaders: { + 'user-agent': /detect-node-support\/.* \(https:\/\/github.com\/pkgjs\/detect-node-support#readme\)/ + } + }) .get('/repos/pkgjs/detect-node-support/contents/package.json') .reply(200, { content: Fs.readFileSync(Path.join(__dirname, '..', 'package.json')).toString('base64') From 503abbbf8b4325659b43232ae1ab72b2ffe4c128 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Sat, 20 Jun 2020 13:19:15 +0300 Subject: [PATCH 18/19] feat: support GH_TOKEN auth; support rate limit retries --- README.md | 4 +++ lib/loader/octokit-wrapper.js | 28 +++++++++++++--- package.json | 1 + test/index.js | 61 +++++++++++++++++++++++++++++++++-- 4 files changed, 86 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index aefb45f..f678331 100644 --- a/README.md +++ b/README.md @@ -2,6 +2,10 @@ List the Node.js versions supported by the package/repository +## Setup + +No setup is required, however if you do not have a `GH_TOKEN` environment limit, you will likely hit a request rate limit on Github API, which may result in very long wait times for retries. + ## Usage (command line) ``` diff --git a/lib/loader/octokit-wrapper.js b/lib/loader/octokit-wrapper.js index 9469f45..f52be68 100644 --- a/lib/loader/octokit-wrapper.js +++ b/lib/loader/octokit-wrapper.js @@ -1,17 +1,35 @@ 'use strict'; const { Octokit } = require('@octokit/rest'); +const { throttling } = require('@octokit/plugin-throttling'); const Constants = require('../constants'); +const Logger = require('../logger'); + + +const internals = { + Octokit: Octokit.plugin(throttling) +}; + exports.create = () => { - const octokit = new Octokit({ - userAgent: Constants.userAgent - }); + const octokit = new internals.Octokit({ + auth: process.env.GH_TOKEN, + userAgent: Constants.userAgent, + throttle: { + onRateLimit: (retryAfter, options) => { + + Logger.warn(['loader'], 'Request quota exceeded for request %s %s. Will retry in %d seconds. Have you set a GH_TOKEN in env?', options.method, options.url, retryAfter); - // @todo: onRateLimit - // @todo: auth + return true; + }, + onAbuseLimit: (retryAfter, options) => { + + return false; + } + } + }); return octokit; }; diff --git a/package.json b/package.json index 1d462af..ad8d203 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ }, "dependencies": { "@npmcli/arborist": "0.0.0-pre.21", + "@octokit/plugin-throttling": "^3.2.2", "@octokit/rest": "^18.0.0", "@pkgjs/nv": "0.0.3", "debug": "^4.1.1", diff --git a/test/index.js b/test/index.js index 7c1c3d6..962f6e3 100644 --- a/test/index.js +++ b/test/index.js @@ -533,10 +533,9 @@ describe('detect-node-support', () => { content: Fs.readFileSync(Path.join(__dirname, '..', 'package.json')).toString('base64') }) .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') - .reply(500); + .reply(500, 'Simulated server error'); - const err = await expect(NodeSupport.detect({ repository: 'git+https://github.com/pkgjs/detect-node-support.git' })).to.reject(); - expect(err.name).to.equal('HttpError'); + await expect(NodeSupport.detect({ repository: 'git+https://github.com/pkgjs/detect-node-support.git' })).to.reject('Simulated server error'); }); it('throws when repository does not have a package.json', async () => { @@ -585,6 +584,62 @@ describe('detect-node-support', () => { await expect(NodeSupport.detect({ repository: 'git+https://github.example.com/pkgjs/detect-node-support.git' })) .to.reject('Only github.com paths supported, feel free to PR at https://github.com/pkgjs/detect-node-support'); }); + + it('retries when rate limited', async () => { + + fixture.stubs.listRemote + .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); + + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', 'package.json')).toString('base64') + }) + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') + .reply(403, null, { + 'x-ratelimit-limit': '60', + 'x-ratelimit-remaining': '0', + 'x-ratelimit-reset': `${Math.round(Date.now() / 1000) + 1}` + }) + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', '.travis.yml')).toString('base64') + }); + + const result = await NodeSupport.detect({ repository: 'git+https://github.com/pkgjs/detect-node-support.git' }); + + expect(result).to.equal({ + name: 'detect-node-support', + version: '0.0.0-development', + commit: '9cef39d21ad229dea4b10295f55b0d9a83800b23', + timestamp: 1580673602000, + travis: { + raw: ['10', '12', '14'], + resolved: { + '10': '10.20.1', + '12': '12.17.0', + '14': '14.3.0' + } + }, + engines: '>=10' + }); + }); + + it('aborts on abuse limit', async () => { + + fixture.stubs.listRemote + .returns('9cef39d21ad229dea4b10295f55b0d9a83800b23\tHEAD\n'); + + Nock('https://api.github.com') + .get('/repos/pkgjs/detect-node-support/contents/package.json') + .reply(200, { + content: Fs.readFileSync(Path.join(__dirname, '..', 'package.json')).toString('base64') + }) + .get('/repos/pkgjs/detect-node-support/contents/.travis.yml') + .reply(403, 'Abuse detected'); + + await expect(NodeSupport.detect({ repository: 'git+https://github.com/pkgjs/detect-node-support.git' })).to.reject(/Abuse detected/); + }); }); describe('packageName', () => { From 178e6a5fc62e553d909422684db36024f2d32259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dominykas=20Blyz=CC=8Ce=CC=87?= Date: Sat, 20 Jun 2020 13:33:12 +0300 Subject: [PATCH 19/19] refactor: extract logger --- bin/detect-node-support | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/bin/detect-node-support b/bin/detect-node-support index 9b79131..c8908ec 100755 --- a/bin/detect-node-support +++ b/bin/detect-node-support @@ -31,7 +31,13 @@ Options: exports.main = async ({ _: [what], deps, deep, dev, json }) => { - Debug.enable('detect-node-support:warn:*,detect-node-support:error:*'); + const enabledLogs = ['detect-node-support:warn:*', 'detect-node-support:error:*']; + + if (process.env.DEBUG) { + enabledLogs.push(process.env.DEBUG); + } + + Debug.enable(enabledLogs.join(',')); if (!what) { console.log(internals.help());