From 424ff042378beeb636f2c1afc4a2184405eff931 Mon Sep 17 00:00:00 2001 From: Phillip Johnsen Date: Thu, 22 Apr 2021 21:15:26 +0200 Subject: [PATCH] labels: disable labelling of PRs as it's replaced by GitHub Action First step of removing labelling of PRs as they are opened. Only the part actively listening for newly opened PRs gets removed now, with the main goal of *stopping* @nodejs-github-bot from applying labels and thereby leaving room for nodejs/node-pr-labeler GitHub Action to apply those labels intead. Still a lot of code that can be removed, like the actually logic of resolving labels which has been extracted into the said GitHub Action, but postponing that to a followup PR, when seeing the Action behaving as expected -- if not, these changes might be reverted as a fallback. Refs https://github.com/nodejs/node-pr-labeler --- scripts/node-subsystem-label.js | 33 --- test/integration/node-labels-webhook.test.js | 200 ------------------- 2 files changed, 233 deletions(-) delete mode 100644 scripts/node-subsystem-label.js delete mode 100644 test/integration/node-labels-webhook.test.js diff --git a/scripts/node-subsystem-label.js b/scripts/node-subsystem-label.js deleted file mode 100644 index ce3bd0b9..00000000 --- a/scripts/node-subsystem-label.js +++ /dev/null @@ -1,33 +0,0 @@ -'use strict' - -const debug = require('debug')('node_subsystem_label') - -const nodeRepo = require('../lib/node-repo') - -const timeoutInSec = process.env.WAIT_SECONDS_BEFORE_RESOLVING_LABELS || 2 - -module.exports = function (app, events) { - events.on('pull_request.opened', handlePrCreated) -} - -function handlePrCreated (event, owner, repo) { - const prId = event.number - const logger = event.logger - const baseBranch = event.pull_request.base.ref - - // subsystem labelling is for node core only - if (repo !== 'node') return - - debug(`/${owner}/${repo}/pull/${prId} opened`) - // by not hard coding the owner repo to nodejs/node here, - // we can test these this script in a different repo than - // *actual* node core as long as the repo is named "node" - return nodeRepo.resolveLabelsThenUpdatePr({ - owner, - repo, - prId, - logger, - baseBranch, - timeoutInSec - }) -} diff --git a/test/integration/node-labels-webhook.test.js b/test/integration/node-labels-webhook.test.js deleted file mode 100644 index 34fac9f1..00000000 --- a/test/integration/node-labels-webhook.test.js +++ /dev/null @@ -1,200 +0,0 @@ -'use strict' - -const tap = require('tap') -const url = require('url') -const nock = require('nock') -const supertest = require('supertest') -const proxyquire = require('proxyquire') - -const testStubs = { - './github-secret': { - isValid: () => true, - - // necessary to make makes proxyquire return this stub - // whenever *any* module tries to require('./github-secret') - '@global': true - } -} - -process.env.WAIT_SECONDS_BEFORE_RESOLVING_LABELS = 0 - -const readFixture = require('../read-fixture') - -// Clearing the require cache is needed due to labels being cached into a singleton variable. -// To ensure every test can run on its own without relying on other tests having run already -// resulted in the cache being filled up, we enforce all tests to run without any "cache warming", -// hence labels has to be fetched every time -function clearRequireCache () { - for (const modulePath of Object.keys(require.cache)) { - delete require.cache[modulePath] - } -} - -function initializeApp () { - const { app, events } = proxyquire('../../app', testStubs) - clearRequireCache() - require('../../scripts/node-subsystem-label')(app, events) - return app -} - -setupNoRequestMatchHandler() - -tap.test('Sends POST request to https://api.github.com/repos/nodejs/node/issues//labels', (t) => { - const app = initializeApp() - const expectedLabels = ['timers'] - const webhookPayload = readFixture('pull-request-opened.json') - - const filesScope = nock('https://api.github.com') - .filteringPath(ignoreQueryParams) - .get('/repos/nodejs/node/pulls/19/files') - .reply(200, readFixture('pull-request-files.json')) - - const existingRepoLabelsScope = nock('https://api.github.com') - .filteringPath(ignoreQueryParams) - .get('/repos/nodejs/node/labels') - .reply(200, readFixture('repo-labels.json')) - - const newLabelsScope = nock('https://api.github.com') - .filteringPath(ignoreQueryParams) - .post('/repos/nodejs/node/issues/19/labels', { labels: expectedLabels }) - .reply(200) - - t.plan(1) - t.tearDown(() => { - filesScope.done() - existingRepoLabelsScope.done() - newLabelsScope.done() - }) - - supertest(app) - .post('/hooks/github') - .set('x-github-event', 'pull_request') - .send(webhookPayload) - .expect(200) - .end((err, res) => { - t.equal(err, null) - }) -}) - -tap.test('Adds v6.x label when PR is targeting the v6.x-staging branch', (t) => { - const app = initializeApp() - const expectedLabels = ['timers', 'v6.x'] - const webhookPayload = readFixture('pull-request-opened-v6.x.json') - - const filesScope = nock('https://api.github.com') - .filteringPath(ignoreQueryParams) - .get('/repos/nodejs/node/pulls/19/files') - .reply(200, readFixture('pull-request-files.json')) - - const existingRepoLabelsScope = nock('https://api.github.com') - .filteringPath(ignoreQueryParams) - .get('/repos/nodejs/node/labels') - .reply(200, readFixture('repo-labels.json')) - - const newLabelsScope = nock('https://api.github.com') - .filteringPath(ignoreQueryParams) - .post('/repos/nodejs/node/issues/19/labels', { labels: expectedLabels }) - .reply(200) - - t.plan(1) - t.tearDown(() => { - filesScope.done() - existingRepoLabelsScope.done() - newLabelsScope.done() - }) - - supertest(app) - .post('/hooks/github') - .set('x-github-event', 'pull_request') - .send(webhookPayload) - .expect(200) - .end((err, res) => { - t.equal(err, null) - }) -}) - -// reported bug: https://github.com/nodejs/github-bot/issues/58 -tap.test('Does not create labels which does not already exist', (t) => { - const app = initializeApp() - const webhookPayload = readFixture('pull-request-opened-mapproxy.json') - - const filesScope = nock('https://api.github.com') - .filteringPath(ignoreQueryParams) - .get('/repos/nodejs/node/pulls/7972/files') - .reply(200, readFixture('pull-request-files-mapproxy.json')) - - const existingRepoLabelsScope = nock('https://api.github.com') - .filteringPath(ignoreQueryParams) - .get('/repos/nodejs/node/labels') - .reply(200, readFixture('repo-labels.json')) - - t.plan(1) - t.tearDown(() => { - filesScope.done() - existingRepoLabelsScope.done() - }) - - supertest(app) - .post('/hooks/github') - .set('x-github-event', 'pull_request') - .send(webhookPayload) - .expect(200) - .end((err, res) => { - t.equal(err, null) - }) -}) - -// reported bug: https://github.com/nodejs/github-bot/issues/92 -tap.test('Adds V8 Engine label when PR has deps/v8 file changes', (t) => { - const app = initializeApp() - const expectedLabels = ['V8 Engine'] - const webhookPayload = readFixture('pull-request-opened-v8.json') - - const filesScope = nock('https://api.github.com') - .filteringPath(ignoreQueryParams) - .get('/repos/nodejs/node/pulls/9422/files') - .reply(200, readFixture('pull-request-files-v8.json')) - - const existingRepoLabelsScope = nock('https://api.github.com') - .filteringPath(ignoreQueryParams) - .get('/repos/nodejs/node/labels') - .reply(200, readFixture('repo-labels.json')) - - const newLabelsScope = nock('https://api.github.com') - .filteringPath(ignoreQueryParams) - .post('/repos/nodejs/node/issues/9422/labels', { labels: expectedLabels }) - .reply(200) - - t.plan(1) - t.tearDown(() => { - filesScope.done() - existingRepoLabelsScope.done() - newLabelsScope.done() - }) - - supertest(app) - .post('/hooks/github') - .set('x-github-event', 'pull_request') - .send(webhookPayload) - .expect(200) - .end((err, res) => { - t.equal(err, null) - }) -}) - -function ignoreQueryParams (pathAndQuery) { - return url.parse(pathAndQuery, true).pathname -} - -// nock doesn't make the tests explode if an unexpected external request is made, -// we therefore have to attach an explicit "no match" handler too make tests fail -// if there's made outgoing request we didn't expect -function setupNoRequestMatchHandler () { - nock.emitter.on('no match', (req) => { - // requests against the app is expected and we shouldn't need to tell nock about it - if (req.hostname === '127.0.0.1') return - - const reqUrl = `${req._headers.host}${req.path}` - throw new Error(`Unexpected request was sent to ${reqUrl}`) - }) -}