From f534c6e1a942b7cd0c9d182e81e2a08a3e1c2ce5 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Thu, 8 Aug 2024 15:09:23 -0400 Subject: [PATCH 01/22] add initial code for preventing multiple suborg config for repos --- .eslintrc.json | 4 +- lib/settings.js | 117 ++++++++++++++++------------ test/unit/lib/settings.test.js | 137 +++++++++++++++++++++++++++++++-- 3 files changed, 200 insertions(+), 58 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index b6721aca..5db37214 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -1,11 +1,11 @@ { "env": { - "browser": true, + "node": true, "commonjs": true, "es2021": true }, "extends": [ - "standard" + "eslint:recommended" ], "parserOptions": { "ecmaVersion": 12 diff --git a/lib/settings.js b/lib/settings.js index 961aa4c6..35724907 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -24,7 +24,7 @@ class Settings { } } - static async syncSubOrgs (nop, context, suborg, repo, config, ref) { + static async syncSubOrgs(nop, context, suborg, repo, config, ref) { const settings = new Settings(nop, context, repo, config, ref, suborg) try { await settings.loadConfigs() @@ -36,7 +36,7 @@ class Settings { } } - static async sync (nop, context, repo, config, ref) { + static async sync(nop, context, repo, config, ref) { const settings = new Settings(nop, context, repo, config, ref) try { await settings.loadConfigs(repo) @@ -51,13 +51,13 @@ class Settings { } } - static async handleError (nop, context, repo, config, ref, nopcommand) { + static async handleError(nop, context, repo, config, ref, nopcommand) { const settings = new Settings(nop, context, repo, config, ref) settings.appendToResults([nopcommand]) await settings.handleResults() } - constructor (nop, context, repo, config, ref, suborg) { + constructor(nop, context, repo, config, ref, suborg) { this.ref = ref this.context = context this.installation_id = context.payload.installation.id @@ -96,7 +96,7 @@ class Settings { } // Create a check in the Admin repo for safe-settings. - async createCheckRun () { + async createCheckRun() { const startTime = new Date() let conclusion = 'success' let details = `Run on: \`${new Date().toISOString()}\`` @@ -142,7 +142,7 @@ class Settings { }) } - logError (msg) { + logError(msg) { this.log.error(msg) this.errors.push({ owner: this.repo.owner, @@ -152,7 +152,7 @@ class Settings { }) } - async handleResults () { + async handleResults() { const { payload } = this.context // Create a checkrun if not in nop mode @@ -226,23 +226,23 @@ class Settings { #### :robot: Safe-Settings config changes detected: ${this.results.reduce((x, y) => { - if (!y) { - return x - } - if (y.type === 'ERROR') { - error = true - return `${x} + if (!y) { + return x + } + if (y.type === 'ERROR') { + error = true + return `${x} ❗ ${y.action.msg} ${y.plugin} ${prettify(y.repo)} ${prettify(y.action.additions)} ${prettify(y.action.deletions)} ${prettify(y.action.modifications)} ` - } else if (y.action.additions === null && y.action.deletions === null && y.action.modifications === null) { - return `${x}` - } else { - if (y.action === undefined) { - return `${x}` - } - return `${x} + } else if (y.action.additions === null && y.action.deletions === null && y.action.modifications === null) { + return `${x}` + } else { + if (y.action === undefined) { + return `${x}` + } + return `${x} ✋ ${y.plugin} ${prettify(y.repo)} ${prettify(y.action.additions)} ${prettify(y.action.deletions)} ${prettify(y.action.modifications)} ` - } -}, table)} + } + }, table)} ` const pullRequest = payload.check_run.check_suite.pull_requests[0] @@ -272,12 +272,12 @@ ${this.results.reduce((x, y) => { await this.github.checks.update(params) } - async loadConfigs (repo) { + async loadConfigs(repo) { this.subOrgConfigs = await this.getSubOrgConfigs() this.repoConfigs = await this.getRepoConfigs(repo) } - async updateOrg () { + async updateOrg() { const rulesetsConfig = this.config.rulesets if (rulesetsConfig) { const RulesetsPlugin = Settings.PLUGINS.rulesets @@ -287,7 +287,7 @@ ${this.results.reduce((x, y) => { } } - async updateRepos (repo) { + async updateRepos(repo) { this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs() let repoConfig = this.config.repository if (repoConfig) { @@ -353,7 +353,7 @@ ${this.results.reduce((x, y) => { } } - async updateAll () { + async updateAll() { // this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs(this.github, this.repo, this.log) // this.repoConfigs = this.repoConfigs || await this.getRepoConfigs(this.github, this.repo, this.log) return this.eachRepositoryRepos(this.github, this.config.restrictedRepos, this.log).then(res => { @@ -361,7 +361,7 @@ ${this.results.reduce((x, y) => { }) } - getSubOrgConfig (repoName) { + getSubOrgConfig(repoName) { if (this.subOrgConfigs) { for (const k of Object.keys(this.subOrgConfigs)) { const repoPattern = new Glob(k) @@ -374,13 +374,13 @@ ${this.results.reduce((x, y) => { } // Remove Org specific configs from the repo config - returnRepoSpecificConfigs (config) { + returnRepoSpecificConfigs(config) { const newConfig = Object.assign({}, config) // clone delete newConfig.rulesets return newConfig } - childPluginsList (repo) { + childPluginsList(repo) { const repoName = repo.repo const subOrgOverrideConfig = this.getSubOrgConfig(repoName) this.log.debug(`suborg config for ${repoName} is ${JSON.stringify(subOrgOverrideConfig)}`) @@ -412,7 +412,7 @@ ${this.results.reduce((x, y) => { return childPlugins } - validate (section, baseConfig, overrideConfig) { + validate(section, baseConfig, overrideConfig) { const configValidator = this.configvalidators[section] if (configValidator) { this.log.debug(`Calling configvalidator for key ${section} `) @@ -431,7 +431,7 @@ ${this.results.reduce((x, y) => { } } - isRestricted (repoName) { + isRestricted(repoName) { const restrictedRepos = this.config.restrictedRepos // Skip configuring any restricted repos if (Array.isArray(restrictedRepos)) { @@ -463,11 +463,11 @@ ${this.results.reduce((x, y) => { return false } - includesRepo (repoName, restrictedRepos) { + includesRepo(repoName, restrictedRepos) { return restrictedRepos.filter((restrictedRepo) => { return RegExp(restrictedRepo).test(repoName) }).length > 0 } - async eachRepositoryRepos (github, restrictedRepos, log) { + async eachRepositoryRepos(github, restrictedRepos, log) { log.debug('Fetching repositories') return github.paginate('GET /installation/repositories').then(repositories => { return Promise.all(repositories.map(repository => { @@ -488,7 +488,7 @@ ${this.results.reduce((x, y) => { * @param params Params to fetch the file with * @return The parsed YAML file */ - async loadConfigMap (params) { + async loadConfigMap(params) { try { this.log.debug(` In loadConfigMap ${JSON.stringify(params)}`) const response = await this.github.repos.getContent(params).catch(e => { @@ -535,7 +535,7 @@ ${this.results.reduce((x, y) => { * @param params Params to fetch the file with * @return The parsed YAML file */ - async getRepoConfigMap () { + async getRepoConfigMap() { try { this.log.debug(` In getRepoConfigMap ${JSON.stringify(this.repo)}`) // GitHub getContent api has a hard limit of returning 1000 entries without @@ -602,7 +602,7 @@ ${this.results.reduce((x, y) => { * @param params Params to fetch the file with * @return The parsed YAML file */ - async getSubOrgConfigMap () { + async getSubOrgConfigMap() { try { this.log.debug(` In getSubOrgConfigMap ${JSON.stringify(this.repo)}`) const repo = { owner: this.repo.owner, repo: env.ADMIN_REPO } @@ -629,7 +629,7 @@ ${this.results.reduce((x, y) => { * @param {*} repo repo param * @returns repoConfigs object */ - async getRepoConfigs (repo) { + async getRepoConfigs(repo) { try { const overridePaths = await this.getRepoConfigMap() const repoConfigs = {} @@ -681,7 +681,7 @@ ${this.results.reduce((x, y) => { * @param params Params to fetch the file with * @return The parsed YAML file */ - async getSubOrgConfigs () { + async getSubOrgConfigs() { try { if (this.subOrgConfigMap) { this.log.debug(`SubOrg config was changed and the associated overridePaths is = ${JSON.stringify(this.subOrgConfigMap)}`) @@ -698,7 +698,19 @@ ${this.results.reduce((x, y) => { subOrgConfigs[override.name] = data if (data.suborgrepos) { data.suborgrepos.forEach(repository => { - subOrgConfigs[repository] = data + this.storeSubOrgConfig(subOrgConfigs, override.path, repository, data) + + // In case support for multiple suborg configs for the same repo is required, merge the configs. + // + // Planned for the future to support multiple suborgrepos for the same repo + // + // if (existingConfigForRepo) { + // subOrgConfigs[repository] = this.mergeDeep.mergeDeep({}, existingConfigForRepo, data) + // } else { + // subOrgConfigs[repository] = data + // } + + subOrgConfigs[repository] = Object.assign({}, data, { source: override.path }) }) } if (data.suborgteams) { @@ -708,7 +720,7 @@ ${this.results.reduce((x, y) => { await Promise.all(promises).then(res => { res.forEach(r => { r.forEach(e => { - subOrgConfigs[e.name] = data + this.storeSubOrgConfig(subOrgConfigs, override.path, e.name, data) }) }) }) @@ -720,7 +732,7 @@ ${this.results.reduce((x, y) => { await Promise.all(promises).then(res => { res.forEach(r => { r.forEach(e => { - subOrgConfigs[e.repository_name] = data + this.storeSubOrgConfig(subOrgConfigs, override.path, e.repository_name, data) }) }) }) @@ -739,13 +751,21 @@ ${this.results.reduce((x, y) => { } } + storeSubOrgConfig(subOrgConfigs, overridePath, repoName, data) { + const existingConfigForRepo = subOrgConfigs[repoName] + if (existingConfigForRepo && existingConfigForRepo.source !== overridePath) { + throw new Error(`Multiple suborg configs for ${repoName} in ${overridePath} and ${existingConfigForRepo?.source}`) + } + subOrgConfigs[repoName] = Object.assign({}, data, { source: overridePath }) + } + /** * Loads a file from GitHub * * @param params Params to fetch the file with * @return The parsed YAML file */ - async loadYaml (filePath) { + async loadYaml(filePath) { try { const repo = { owner: this.repo.owner, repo: env.ADMIN_REPO } const params = Object.assign(repo, { path: filePath, ref: this.ref }) @@ -782,13 +802,13 @@ ${this.results.reduce((x, y) => { } } - appendToResults (res) { + appendToResults(res) { if (this.nop) { this.results = this.results.concat(res.flat(3)) } } - async getReposForTeam (teamslug) { + async getReposForTeam(teamslug) { const options = this.github.rest.teams.listReposInOrg.endpoint.merge({ org: this.repo.owner, team_slug: teamslug, @@ -797,20 +817,19 @@ ${this.results.reduce((x, y) => { return this.github.paginate(options) } - async getReposForCustomProperty (customPropertyTuple) { - const name=Object.keys(customPropertyTuple)[0] + async getReposForCustomProperty(customPropertyTuple) { + const name = Object.keys(customPropertyTuple)[0] let q = `props.${name}:${customPropertyTuple[name]}` q = encodeURIComponent(q) const options = this.github.request.endpoint((`/orgs/${this.repo.owner}/properties/values?repository_query=${q}`)) return this.github.paginate(options) } - - isObject (item) { + isObject(item) { return (item && typeof item === 'object' && !Array.isArray(item)) } - isIterable (obj) { + isIterable(obj) { // checks for null and undefined if (obj == null) { return false diff --git a/test/unit/lib/settings.test.js b/test/unit/lib/settings.test.js index c289f563..c91583d3 100644 --- a/test/unit/lib/settings.test.js +++ b/test/unit/lib/settings.test.js @@ -1,6 +1,13 @@ /* eslint-disable no-undef */ - +const { Octokit } = require('octokit') const Settings = require('../../../lib/settings') +const yaml = require('js-yaml') +// jest.mock('../../../lib/settings', () => { +// const OriginalSettings = jest.requireActual('../../../lib/settings') +// //const orginalSettingsInstance = new OriginalSettings(false, stubContext, mockRepo, config, mockRef, mockSubOrg) +// return OriginalSettings +// }) + describe('Settings Tests', () => { let stubContext @@ -8,19 +15,61 @@ describe('Settings Tests', () => { let stubConfig let mockRef let mockSubOrg + let subOrgConfig - function createSettings (config) { - return new Settings(false, stubContext, mockRepo, config, mockRef, mockSubOrg) + function createSettings(config) { + const settings = new Settings(false, stubContext, mockRepo, config, mockRef, mockSubOrg) + return settings; } beforeEach(() => { + const mockOctokit = jest.mocked(Octokit) + const content = Buffer.from(` +suborgrepos: +- new-repo +#- test* +#- secret* + +suborgteams: +- core + +suborgproperties: +- EDP: true +- do_no_delete: true + +teams: + - name: core + permission: bypass + - name: docss + permission: pull + - name: docs + permission: pull + +validator: + pattern: '[a-zA-Z0-9_-]+_[a-zA-Z0-9_-]+.*' + +repository: + # A comma-separated list of topics to set on the repository + topics: + - frontend + `).toString('base64'); + mockOctokit.repos = { + getContent: jest.fn().mockResolvedValue({ data: { content } }) + } + + mockOctokit.request = { + endpoint: jest.fn().mockReturnValue({}) + } + + mockOctokit.paginate = jest.fn().mockResolvedValue([]) + stubContext = { payload: { installation: { id: 123 } }, - octokit: jest.fn(), + octokit: mockOctokit, log: { debug: jest.fn((msg) => { console.log(msg) @@ -34,9 +83,11 @@ describe('Settings Tests', () => { } } - mockRepo = jest.fn() - mockRef = jest.fn() - mockSubOrg = jest.fn() + + + mockRepo = { owner: 'test', repo: 'test-repo' } + mockRef = 'main' + mockSubOrg = 'frontend' }) describe('restrictedRepos', () => { @@ -140,4 +191,76 @@ describe('Settings Tests', () => { }) }) }) // restrictedRepos + + describe('loadConfigs', () => { + describe('load suborg configs', () => { + beforeEach(() => { + stubConfig = { + restrictedRepos: { + } + } + subOrgConfig = yaml.load(` + suborgrepos: + - new-repo + + suborgproperties: + - EDP: true + - do_no_delete: true + + teams: + - name: core + permission: bypass + - name: docss + permission: pull + - name: docs + permission: pull + + validator: + pattern: '[a-zA-Z0-9_-]+_[a-zA-Z0-9_-]+.*' + + repository: + # A comma-separated list of topics to set on the repository + topics: + - frontend + + `) + + }) + + it("Should load configMap for suborgs'", async () => { + //mockSubOrg = jest.fn().mockReturnValue(['suborg1', 'suborg2']) + mockSubOrg = undefined + settings = createSettings(stubConfig) + jest.spyOn(settings, 'loadConfigMap').mockImplementation(() => [{ name: "frontend", path: ".github/suborgs/frontend.yml" }]) + jest.spyOn(settings, 'loadYaml').mockImplementation(() => subOrgConfig) + jest.spyOn(settings, 'getReposForTeam').mockImplementation(() => [{ name: 'repo-test' }]) + jest.spyOn(settings, 'getReposForCustomProperty').mockImplementation(() => [{ repository_name: 'repo-for-property' }]) + + const subOrgConfigs = await settings.getSubOrgConfigs() + expect(settings.loadConfigMap).toHaveBeenCalledTimes(1) + + // Get own properties of subOrgConfigs + const ownProperties = Object.getOwnPropertyNames(subOrgConfigs); + expect(ownProperties.length).toEqual(3) + }) + + it("Should throw an error when a repo is found in multiple suborgs configs'", async () => { + //mockSubOrg = jest.fn().mockReturnValue(['suborg1', 'suborg2']) + mockSubOrg = undefined + settings = createSettings(stubConfig) + jest.spyOn(settings, 'loadConfigMap').mockImplementation(() => [{ name: "frontend", path: ".github/suborgs/frontend.yml" }, { name: "backend", path: ".github/suborgs/backend.yml" }]) + jest.spyOn(settings, 'loadYaml').mockImplementation(() => subOrgConfig) + jest.spyOn(settings, 'getReposForTeam').mockImplementation(() => [{ name: 'repo-test' }]) + jest.spyOn(settings, 'getReposForCustomProperty').mockImplementation(() => [{ repository_name: 'repo-for-property' }]) + + expect(async () => await settings.getSubOrgConfigs()).rejects.toThrow('Multiple suborg configs for new-repo in .github/suborgs/backend.yml and .github/suborgs/frontend.yml') + // try { + // await settings.getSubOrgConfigs() + // } catch (e) { + // console.log(e) + // } + }) + }) + }) // loadConfigs + }) // Settings Tests From 04434a29683991764824a3c08ab2dc0a4ed37509 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Thu, 29 Aug 2024 16:59:39 -0400 Subject: [PATCH 02/22] detect conflict even when a single suborg config is changed --- lib/settings.js | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/lib/settings.js b/lib/settings.js index 35724907..578d83c7 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -683,10 +683,9 @@ ${this.results.reduce((x, y) => { */ async getSubOrgConfigs() { try { - if (this.subOrgConfigMap) { - this.log.debug(`SubOrg config was changed and the associated overridePaths is = ${JSON.stringify(this.subOrgConfigMap)}`) - } - const overridePaths = this.subOrgConfigMap || await this.getSubOrgConfigMap() + // Get all suborg configs even though we might be here becuase of a suborg config change + // we will filter them out if request is due to a suborg config change + const overridePaths = await this.getSubOrgConfigMap() const subOrgConfigs = {} for (const override of overridePaths) { @@ -698,7 +697,7 @@ ${this.results.reduce((x, y) => { subOrgConfigs[override.name] = data if (data.suborgrepos) { data.suborgrepos.forEach(repository => { - this.storeSubOrgConfig(subOrgConfigs, override.path, repository, data) + this.storeSubOrgConfigIfNoConflicts(subOrgConfigs, override.path, repository, data) // In case support for multiple suborg configs for the same repo is required, merge the configs. // @@ -720,7 +719,7 @@ ${this.results.reduce((x, y) => { await Promise.all(promises).then(res => { res.forEach(r => { r.forEach(e => { - this.storeSubOrgConfig(subOrgConfigs, override.path, e.name, data) + this.storeSubOrgConfigIfNoConflicts(subOrgConfigs, override.path, e.name, data) }) }) }) @@ -732,12 +731,26 @@ ${this.results.reduce((x, y) => { await Promise.all(promises).then(res => { res.forEach(r => { r.forEach(e => { - this.storeSubOrgConfig(subOrgConfigs, override.path, e.repository_name, data) + this.storeSubOrgConfigIfNoConflicts(subOrgConfigs, override.path, e.repository_name, data) }) }) }) } } + + // If this was result of a suborg config change, only return the repos that are part of the suborg config + if (this.subOrgConfigMap) { + this.log.debug(`SubOrg config was changed and the associated overridePaths is = ${JSON.stringify(this.subOrgConfigMap)}`) + // enumerate the properties of the subOrgConfigs object and delete the ones that are not part of the suborg + for (const [key, value] of Object.entries(subOrgConfigs)) { + if (!this.subOrgConfigMap.some((overridePath) => { + return overridePath.path === value.source + } + )) { + delete subOrgConfigs[key] + } + } + } return subOrgConfigs } catch (e) { if (this.nop) { @@ -751,7 +764,7 @@ ${this.results.reduce((x, y) => { } } - storeSubOrgConfig(subOrgConfigs, overridePath, repoName, data) { + storeSubOrgConfigIfNoConflicts(subOrgConfigs, overridePath, repoName, data) { const existingConfigForRepo = subOrgConfigs[repoName] if (existingConfigForRepo && existingConfigForRepo.source !== overridePath) { throw new Error(`Multiple suborg configs for ${repoName} in ${overridePath} and ${existingConfigForRepo?.source}`) From 775b7cac20e69b3324d41070ce6c459b00d681c8 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Tue, 17 Sep 2024 07:49:36 -0400 Subject: [PATCH 03/22] remove duplicate errors --- lib/settings.js | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/settings.js b/lib/settings.js index 578d83c7..375fafb8 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -162,6 +162,16 @@ class Settings { return } + //remove nulls and undefined in the results + this.results = this.results.filter((thing) => thing ? true : false) + + //remove duplicate rows in this.results + this.results = this.results.filter((thing, index, self) => { + return index === self.findIndex((t) => { + return t.type === thing.type && t.repo === thing.repo && t.plugin === thing.plugin + }) + }) + let error = false // Different logic const stats = { From 60f8fd37ad49efb16f1f1f28c83ffd8fe7c38e67 Mon Sep 17 00:00:00 2001 From: Yadhav Jayaraman <57544838+decyjphr@users.noreply.github.com> Date: Tue, 17 Sep 2024 09:45:08 -0400 Subject: [PATCH 04/22] dont add nulls and undefined to results --- lib/settings.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/settings.js b/lib/settings.js index 375fafb8..9683cb82 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -162,9 +162,6 @@ class Settings { return } - //remove nulls and undefined in the results - this.results = this.results.filter((thing) => thing ? true : false) - //remove duplicate rows in this.results this.results = this.results.filter((thing, index, self) => { return index === self.findIndex((t) => { @@ -827,7 +824,10 @@ ${this.results.reduce((x, y) => { appendToResults(res) { if (this.nop) { - this.results = this.results.concat(res.flat(3)) + //Remove nulls and undefined from the results + const results = res.flat(3).filter(r => r) + + this.results = this.results.concat(results) } } From 336d7396cf8750d1f0552a0ef9590c34e1be2433 Mon Sep 17 00:00:00 2001 From: Torgeir S <80778551+Gramatus@users.noreply.github.com> Date: Sun, 15 Sep 2024 13:21:33 +0000 Subject: [PATCH 05/22] add some JSDoc types These were added as part of understanding the code, commiting in case they might be useful for others as well. --- index.js | 14 +++++++++----- lib/settings.js | 23 +++++++++++++++++++++-- test/unit/index.test.js | 5 ++++- 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/index.js b/index.js index 7e4ef032..a607aa4e 100644 --- a/index.js +++ b/index.js @@ -9,7 +9,11 @@ const env = require('./lib/env') let deploymentConfig - +/** + * @import { Probot, ApplicationFunctionOptions, ProbotOctokit } from "probot" + * @param {Probot} robot + * @param {ApplicationFunctionOptions} probotOptions + */ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => { let appName = 'safe-settings' let appSlug = 'safe-settings' @@ -455,15 +459,15 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } else { robot.log.error(error) } - } + } } catch (error) { if (error.status === 404) { //nop - } else { + } else { robot.log.error(error) } - } + } return } else { robot.log.debug('Repository Edited by a Human') @@ -663,7 +667,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => syncInstallation() }) } - + // Get info about the app info() diff --git a/lib/settings.js b/lib/settings.js index 9683cb82..f05c413d 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -9,7 +9,14 @@ const env = require('./env') const CONFIG_PATH = env.CONFIG_PATH const eta = new Eta({ views: path.join(__dirname) }) const SCOPE = { ORG: 'org', REPO: 'repo' } // Determine if the setting is a org setting or repo setting +/** @import { Context, ProbotOctokit } from "probot" */ class Settings { + /** @type {InstanceType} */ + github + /** + * @param {boolean} nop + * @param {Context} context + */ static async syncAll (nop, context, repo, config, ref) { const settings = new Settings(nop, context, repo, config, ref) try { @@ -24,6 +31,10 @@ class Settings { } } + /** + * @param {boolean} nop + * @param {Context} context + */ static async syncSubOrgs(nop, context, suborg, repo, config, ref) { const settings = new Settings(nop, context, repo, config, ref, suborg) try { @@ -36,6 +47,10 @@ class Settings { } } + /** + * @param {boolean} nop + * @param {Context} context + */ static async sync(nop, context, repo, config, ref) { const settings = new Settings(nop, context, repo, config, ref) try { @@ -57,6 +72,10 @@ class Settings { await settings.handleResults() } + /** + * @param {boolean} nop + * @param {Context} context + */ constructor(nop, context, repo, config, ref, suborg) { this.ref = ref this.context = context @@ -755,7 +774,7 @@ ${this.results.reduce((x, y) => { } )) { delete subOrgConfigs[key] - } + } } } return subOrgConfigs @@ -826,7 +845,7 @@ ${this.results.reduce((x, y) => { if (this.nop) { //Remove nulls and undefined from the results const results = res.flat(3).filter(r => r) - + this.results = this.results.concat(results) } } diff --git a/test/unit/index.test.js b/test/unit/index.test.js index 9d09804d..a5c70a95 100644 --- a/test/unit/index.test.js +++ b/test/unit/index.test.js @@ -1,8 +1,11 @@ const { Probot, ProbotOctokit } = require('probot') const plugin = require('../../index') +/** @import { ProbotOctokit } from "probot" */ describe.skip('plugin', () => { - let app, event, sync, github + let app, event, sync + /** @type {InstanceType} */ + let github beforeEach(() => { class Octokit { From e826f4066228b695db60523f9c3ea45f02f6b695 Mon Sep 17 00:00:00 2001 From: Torgeir S <80778551+Gramatus@users.noreply.github.com> Date: Sun, 15 Sep 2024 18:36:44 +0000 Subject: [PATCH 06/22] fix some buggy logging statements --- index.js | 2 +- lib/plugins/rulesets.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/index.js b/index.js index a607aa4e..fdffe6b9 100644 --- a/index.js +++ b/index.js @@ -387,7 +387,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => robot.on('repository.edited', async context => { const { payload } = context const { sender } = payload - robot.log.debug('repository.edited payload from ', JSON.stringify(sender)) + robot.log.debug(sender, 'repository.edited payload from') if (sender.type === 'Bot') { robot.log.debug('Repository Edited by a Bot') diff --git a/lib/plugins/rulesets.js b/lib/plugins/rulesets.js index 6846e1ce..f450846d 100644 --- a/lib/plugins/rulesets.js +++ b/lib/plugins/rulesets.js @@ -20,13 +20,13 @@ module.exports = class Rulesets extends Diffable { // Find all Rulesets for this org find () { if (this.scope === 'org') { - this.log.debug(`Getting all rulesets for the org ${this.org}`) + this.log.debug(`Getting all rulesets for the org ${this.repo.owner}`) const listOptions = this.github.request.endpoint.merge('GET /orgs/{org}/rulesets', { org: this.repo.owner, headers: version }) - this.log(listOptions) + this.log.debug(listOptions) return this.github.paginate(listOptions) .then(res => { const rulesets = res.map(ruleset => { From 2b90f8bced6e6e124dd9cd8fde498a7a21116f0a Mon Sep 17 00:00:00 2001 From: Torgeir S <80778551+Gramatus@users.noreply.github.com> Date: Sun, 15 Sep 2024 18:37:41 +0000 Subject: [PATCH 07/22] add missing owner.login property in push.settings.json This is what probot actually uses to identify the org, so it should probably be there in the fixture. --- test/fixtures/events/push.settings.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/fixtures/events/push.settings.json b/test/fixtures/events/push.settings.json index d12de4c2..9c080cba 100644 --- a/test/fixtures/events/push.settings.json +++ b/test/fixtures/events/push.settings.json @@ -61,7 +61,8 @@ "full_name": "bkeepers-inc/botland", "owner": { "name": "bkeepers-inc", - "email": null + "email": null, + "login": "bkeepers-inc" }, "private": true, "html_url": "https://github.com/bkeepers-inc/botland", From f94f6e413ca56b9a0cb0097e768d46524ed27c41 Mon Sep 17 00:00:00 2001 From: Torgeir S <80778551+Gramatus@users.noreply.github.com> Date: Sun, 15 Sep 2024 18:38:46 +0000 Subject: [PATCH 08/22] change sender type in repository.edited.json When `sender.type` is bot, the change is skipped. This led to the test checking for changes on this event failing. --- test/fixtures/events/repository.edited.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/events/repository.edited.json b/test/fixtures/events/repository.edited.json index 42c0068e..92ee6791 100644 --- a/test/fixtures/events/repository.edited.json +++ b/test/fixtures/events/repository.edited.json @@ -130,7 +130,7 @@ "repos_url": "https://api.github.com/users/_safe_settings%5Bbot%5D/repos", "events_url": "https://api.github.com/users/_safe_settings%5Bbot%5D/events{/privacy}", "received_events_url": "https://api.github.com/users/_safe_settings%5Bbot%5D/received_events", - "type": "Bot", + "type": "NotBot", "site_admin": false }, "installation": { From 520f6e828665d26ef86d799c60782e6aacb504e0 Mon Sep 17 00:00:00 2001 From: Torgeir S <80778551+Gramatus@users.noreply.github.com> Date: Sun, 15 Sep 2024 18:43:47 +0000 Subject: [PATCH 09/22] use standard probot logger in index.test.js This seems to me to be the best way for logs to show up in a useful fashion when running tests. --- test/unit/index.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/index.test.js b/test/unit/index.test.js index a5c70a95..99982205 100644 --- a/test/unit/index.test.js +++ b/test/unit/index.test.js @@ -1,4 +1,5 @@ -const { Probot, ProbotOctokit } = require('probot') +const { Probot } = require('probot') +const { getLog } = require('probot/lib/helpers/get-log') const plugin = require('../../index') /** @import { ProbotOctokit } from "probot" */ @@ -27,14 +28,13 @@ describe.skip('plugin', () => { } } - app = new Probot({ secret: "abcdef", Octokit }) + app = new Probot({ secret: 'abcdef', Octokit, log: getLog({ level: 'info' }) }) github = { repos: { getContents: jest.fn(() => Promise.resolve({ data: { content: '' } })) } } app.auth = () => Promise.resolve(github) - app.log = { debug: jest.fn(), error: console.error } event = { name: 'push', payload: JSON.parse(JSON.stringify(require('../fixtures/events/push.settings.json'))) From 6eff20c84b5e28844ff9e34072232b3f4c2a6284 Mon Sep 17 00:00:00 2001 From: Torgeir S <80778551+Gramatus@users.noreply.github.com> Date: Sun, 15 Sep 2024 18:45:54 +0000 Subject: [PATCH 10/22] fix index.test.js --- .../api_responses/get_repository.json | 521 ++++++++++++++++++ test/fixtures/settings.yml | 195 +++++++ test/unit/index.test.js | 108 +++- 3 files changed, 801 insertions(+), 23 deletions(-) create mode 100644 test/fixtures/api_responses/get_repository.json create mode 100644 test/fixtures/settings.yml diff --git a/test/fixtures/api_responses/get_repository.json b/test/fixtures/api_responses/get_repository.json new file mode 100644 index 00000000..f46611bd --- /dev/null +++ b/test/fixtures/api_responses/get_repository.json @@ -0,0 +1,521 @@ +{ + "id": 1296269, + "node_id": "MDEwOlJlcG9zaXRvcnkxMjk2MjY5", + "name": "Hello-World", + "full_name": "octocat/Hello-World", + "owner": { + "login": "octocat", + "id": 1, + "node_id": "MDQ6VXNlcjE=", + "avatar_url": "https://github.com/images/error/octocat_happy.gif", + "gravatar_id": "", + "url": "https://api.github.com/users/octocat", + "html_url": "https://github.com/octocat", + "followers_url": "https://api.github.com/users/octocat/followers", + "following_url": "https://api.github.com/users/octocat/following{/other_user}", + "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}", + "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/octocat/subscriptions", + "organizations_url": "https://api.github.com/users/octocat/orgs", + "repos_url": "https://api.github.com/users/octocat/repos", + "events_url": "https://api.github.com/users/octocat/events{/privacy}", + "received_events_url": "https://api.github.com/users/octocat/received_events", + "type": "User", + "site_admin": false + }, + "private": false, + "html_url": "https://github.com/octocat/Hello-World", + "description": "This your first repo!", + "fork": false, + "url": "https://api.github.com/repos/octocat/Hello-World", + "archive_url": "https://api.github.com/repos/octocat/Hello-World/{archive_format}{/ref}", + "assignees_url": "https://api.github.com/repos/octocat/Hello-World/assignees{/user}", + "blobs_url": "https://api.github.com/repos/octocat/Hello-World/git/blobs{/sha}", + "branches_url": "https://api.github.com/repos/octocat/Hello-World/branches{/branch}", + "collaborators_url": "https://api.github.com/repos/octocat/Hello-World/collaborators{/collaborator}", + "comments_url": "https://api.github.com/repos/octocat/Hello-World/comments{/number}", + "commits_url": "https://api.github.com/repos/octocat/Hello-World/commits{/sha}", + "compare_url": "https://api.github.com/repos/octocat/Hello-World/compare/{base}...{head}", + "contents_url": "https://api.github.com/repos/octocat/Hello-World/contents/{+path}", + "contributors_url": "https://api.github.com/repos/octocat/Hello-World/contributors", + "deployments_url": "https://api.github.com/repos/octocat/Hello-World/deployments", + "downloads_url": "https://api.github.com/repos/octocat/Hello-World/downloads", + "events_url": "https://api.github.com/repos/octocat/Hello-World/events", + "forks_url": "https://api.github.com/repos/octocat/Hello-World/forks", + "git_commits_url": "https://api.github.com/repos/octocat/Hello-World/git/commits{/sha}", + "git_refs_url": "https://api.github.com/repos/octocat/Hello-World/git/refs{/sha}", + "git_tags_url": "https://api.github.com/repos/octocat/Hello-World/git/tags{/sha}", + "git_url": "git:github.com/octocat/Hello-World.git", + "issue_comment_url": "https://api.github.com/repos/octocat/Hello-World/issues/comments{/number}", + "issue_events_url": "https://api.github.com/repos/octocat/Hello-World/issues/events{/number}", + "issues_url": "https://api.github.com/repos/octocat/Hello-World/issues{/number}", + "keys_url": "https://api.github.com/repos/octocat/Hello-World/keys{/key_id}", + "labels_url": "https://api.github.com/repos/octocat/Hello-World/labels{/name}", + "languages_url": "https://api.github.com/repos/octocat/Hello-World/languages", + "merges_url": "https://api.github.com/repos/octocat/Hello-World/merges", + "milestones_url": "https://api.github.com/repos/octocat/Hello-World/milestones{/number}", + "notifications_url": "https://api.github.com/repos/octocat/Hello-World/notifications{?since,all,participating}", + "pulls_url": "https://api.github.com/repos/octocat/Hello-World/pulls{/number}", + "releases_url": "https://api.github.com/repos/octocat/Hello-World/releases{/id}", + "ssh_url": "git@github.com:octocat/Hello-World.git", + "stargazers_url": "https://api.github.com/repos/octocat/Hello-World/stargazers", + "statuses_url": "https://api.github.com/repos/octocat/Hello-World/statuses/{sha}", + "subscribers_url": "https://api.github.com/repos/octocat/Hello-World/subscribers", + "subscription_url": "https://api.github.com/repos/octocat/Hello-World/subscription", + "tags_url": "https://api.github.com/repos/octocat/Hello-World/tags", + "teams_url": "https://api.github.com/repos/octocat/Hello-World/teams", + "trees_url": "https://api.github.com/repos/octocat/Hello-World/git/trees{/sha}", + "clone_url": "https://github.com/octocat/Hello-World.git", + "mirror_url": "git:git.example.com/octocat/Hello-World", + "hooks_url": "https://api.github.com/repos/octocat/Hello-World/hooks", + "svn_url": "https://svn.github.com/octocat/Hello-World", + "homepage": "https://github.com", + "forks_count": 9, + "forks": 9, + "stargazers_count": 80, + "watchers_count": 80, + "watchers": 80, + "size": 108, + "default_branch": "master", + "open_issues_count": 0, + "open_issues": 0, + "is_template": false, + "topics": [ + "octocat", + "atom", + "electron", + "api" + ], + "has_issues": true, + "has_projects": true, + "has_wiki": true, + "has_pages": false, + "has_downloads": true, + "has_discussions": false, + "archived": false, + "disabled": false, + "visibility": "public", + "pushed_at": "2011-01-26T19:06:43Z", + "created_at": "2011-01-26T19:01:12Z", + "updated_at": "2011-01-26T19:14:43Z", + "permissions": { + "pull": true, + "push": false, + "admin": false + }, + "allow_rebase_merge": true, + "template_repository": { + "id": 1296269, + "node_id": "MDEwOlJlcG9zaXRvcnkxMjk2MjY5", + "name": "Hello-World-Template", + "full_name": "octocat/Hello-World-Template", + "owner": { + "login": "octocat", + "id": 1, + "node_id": "MDQ6VXNlcjE=", + "avatar_url": "https://github.com/images/error/octocat_happy.gif", + "gravatar_id": "", + "url": "https://api.github.com/users/octocat", + "html_url": "https://github.com/octocat", + "followers_url": "https://api.github.com/users/octocat/followers", + "following_url": "https://api.github.com/users/octocat/following{/other_user}", + "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}", + "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/octocat/subscriptions", + "organizations_url": "https://api.github.com/users/octocat/orgs", + "repos_url": "https://api.github.com/users/octocat/repos", + "events_url": "https://api.github.com/users/octocat/events{/privacy}", + "received_events_url": "https://api.github.com/users/octocat/received_events", + "type": "User", + "site_admin": false + }, + "private": false, + "html_url": "https://github.com/octocat/Hello-World-Template", + "description": "This your first repo!", + "fork": false, + "url": "https://api.github.com/repos/octocat/Hello-World-Template", + "archive_url": "https://api.github.com/repos/octocat/Hello-World-Template/{archive_format}{/ref}", + "assignees_url": "https://api.github.com/repos/octocat/Hello-World-Template/assignees{/user}", + "blobs_url": "https://api.github.com/repos/octocat/Hello-World-Template/git/blobs{/sha}", + "branches_url": "https://api.github.com/repos/octocat/Hello-World-Template/branches{/branch}", + "collaborators_url": "https://api.github.com/repos/octocat/Hello-World-Template/collaborators{/collaborator}", + "comments_url": "https://api.github.com/repos/octocat/Hello-World-Template/comments{/number}", + "commits_url": "https://api.github.com/repos/octocat/Hello-World-Template/commits{/sha}", + "compare_url": "https://api.github.com/repos/octocat/Hello-World-Template/compare/{base}...{head}", + "contents_url": "https://api.github.com/repos/octocat/Hello-World-Template/contents/{+path}", + "contributors_url": "https://api.github.com/repos/octocat/Hello-World-Template/contributors", + "deployments_url": "https://api.github.com/repos/octocat/Hello-World-Template/deployments", + "downloads_url": "https://api.github.com/repos/octocat/Hello-World-Template/downloads", + "events_url": "https://api.github.com/repos/octocat/Hello-World-Template/events", + "forks_url": "https://api.github.com/repos/octocat/Hello-World-Template/forks", + "git_commits_url": "https://api.github.com/repos/octocat/Hello-World-Template/git/commits{/sha}", + "git_refs_url": "https://api.github.com/repos/octocat/Hello-World-Template/git/refs{/sha}", + "git_tags_url": "https://api.github.com/repos/octocat/Hello-World-Template/git/tags{/sha}", + "git_url": "git:github.com/octocat/Hello-World-Template.git", + "issue_comment_url": "https://api.github.com/repos/octocat/Hello-World-Template/issues/comments{/number}", + "issue_events_url": "https://api.github.com/repos/octocat/Hello-World-Template/issues/events{/number}", + "issues_url": "https://api.github.com/repos/octocat/Hello-World-Template/issues{/number}", + "keys_url": "https://api.github.com/repos/octocat/Hello-World-Template/keys{/key_id}", + "labels_url": "https://api.github.com/repos/octocat/Hello-World-Template/labels{/name}", + "languages_url": "https://api.github.com/repos/octocat/Hello-World-Template/languages", + "merges_url": "https://api.github.com/repos/octocat/Hello-World-Template/merges", + "milestones_url": "https://api.github.com/repos/octocat/Hello-World-Template/milestones{/number}", + "notifications_url": "https://api.github.com/repos/octocat/Hello-World-Template/notifications{?since,all,participating}", + "pulls_url": "https://api.github.com/repos/octocat/Hello-World-Template/pulls{/number}", + "releases_url": "https://api.github.com/repos/octocat/Hello-World-Template/releases{/id}", + "ssh_url": "git@github.com:octocat/Hello-World-Template.git", + "stargazers_url": "https://api.github.com/repos/octocat/Hello-World-Template/stargazers", + "statuses_url": "https://api.github.com/repos/octocat/Hello-World-Template/statuses/{sha}", + "subscribers_url": "https://api.github.com/repos/octocat/Hello-World-Template/subscribers", + "subscription_url": "https://api.github.com/repos/octocat/Hello-World-Template/subscription", + "tags_url": "https://api.github.com/repos/octocat/Hello-World-Template/tags", + "teams_url": "https://api.github.com/repos/octocat/Hello-World-Template/teams", + "trees_url": "https://api.github.com/repos/octocat/Hello-World-Template/git/trees{/sha}", + "clone_url": "https://github.com/octocat/Hello-World-Template.git", + "mirror_url": "git:git.example.com/octocat/Hello-World-Template", + "hooks_url": "https://api.github.com/repos/octocat/Hello-World-Template/hooks", + "svn_url": "https://svn.github.com/octocat/Hello-World-Template", + "homepage": "https://github.com", + "language": null, + "forks": 9, + "forks_count": 9, + "stargazers_count": 80, + "watchers_count": 80, + "watchers": 80, + "size": 108, + "default_branch": "master", + "open_issues": 0, + "open_issues_count": 0, + "is_template": true, + "license": { + "key": "mit", + "name": "MIT License", + "url": "https://api.github.com/licenses/mit", + "spdx_id": "MIT", + "node_id": "MDc6TGljZW5zZW1pdA==", + "html_url": "https://api.github.com/licenses/mit" + }, + "topics": [ + "octocat", + "atom", + "electron", + "api" + ], + "has_issues": true, + "has_projects": true, + "has_wiki": true, + "has_pages": false, + "has_downloads": true, + "archived": false, + "disabled": false, + "visibility": "public", + "pushed_at": "2011-01-26T19:06:43Z", + "created_at": "2011-01-26T19:01:12Z", + "updated_at": "2011-01-26T19:14:43Z", + "permissions": { + "admin": false, + "push": false, + "pull": true + }, + "allow_rebase_merge": true, + "temp_clone_token": "ABTLWHOULUVAXGTRYU7OC2876QJ2O", + "allow_squash_merge": true, + "allow_auto_merge": false, + "delete_branch_on_merge": true, + "allow_merge_commit": true, + "subscribers_count": 42, + "network_count": 0 + }, + "temp_clone_token": "ABTLWHOULUVAXGTRYU7OC2876QJ2O", + "allow_squash_merge": true, + "allow_auto_merge": false, + "delete_branch_on_merge": true, + "allow_merge_commit": true, + "allow_forking": true, + "subscribers_count": 42, + "network_count": 0, + "license": { + "key": "mit", + "name": "MIT License", + "spdx_id": "MIT", + "url": "https://api.github.com/licenses/mit", + "node_id": "MDc6TGljZW5zZW1pdA==" + }, + "organization": { + "login": "octocat", + "id": 1, + "node_id": "MDQ6VXNlcjE=", + "avatar_url": "https://github.com/images/error/octocat_happy.gif", + "gravatar_id": "", + "url": "https://api.github.com/users/octocat", + "html_url": "https://github.com/octocat", + "followers_url": "https://api.github.com/users/octocat/followers", + "following_url": "https://api.github.com/users/octocat/following{/other_user}", + "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}", + "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/octocat/subscriptions", + "organizations_url": "https://api.github.com/users/octocat/orgs", + "repos_url": "https://api.github.com/users/octocat/repos", + "events_url": "https://api.github.com/users/octocat/events{/privacy}", + "received_events_url": "https://api.github.com/users/octocat/received_events", + "type": "Organization", + "site_admin": false + }, + "parent": { + "id": 1296269, + "node_id": "MDEwOlJlcG9zaXRvcnkxMjk2MjY5", + "name": "Hello-World", + "full_name": "octocat/Hello-World", + "owner": { + "login": "octocat", + "id": 1, + "node_id": "MDQ6VXNlcjE=", + "avatar_url": "https://github.com/images/error/octocat_happy.gif", + "gravatar_id": "", + "url": "https://api.github.com/users/octocat", + "html_url": "https://github.com/octocat", + "followers_url": "https://api.github.com/users/octocat/followers", + "following_url": "https://api.github.com/users/octocat/following{/other_user}", + "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}", + "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/octocat/subscriptions", + "organizations_url": "https://api.github.com/users/octocat/orgs", + "repos_url": "https://api.github.com/users/octocat/repos", + "events_url": "https://api.github.com/users/octocat/events{/privacy}", + "received_events_url": "https://api.github.com/users/octocat/received_events", + "type": "User", + "site_admin": false + }, + "private": false, + "html_url": "https://github.com/octocat/Hello-World", + "description": "This your first repo!", + "fork": false, + "url": "https://api.github.com/repos/octocat/Hello-World", + "archive_url": "https://api.github.com/repos/octocat/Hello-World/{archive_format}{/ref}", + "assignees_url": "https://api.github.com/repos/octocat/Hello-World/assignees{/user}", + "blobs_url": "https://api.github.com/repos/octocat/Hello-World/git/blobs{/sha}", + "branches_url": "https://api.github.com/repos/octocat/Hello-World/branches{/branch}", + "collaborators_url": "https://api.github.com/repos/octocat/Hello-World/collaborators{/collaborator}", + "comments_url": "https://api.github.com/repos/octocat/Hello-World/comments{/number}", + "commits_url": "https://api.github.com/repos/octocat/Hello-World/commits{/sha}", + "compare_url": "https://api.github.com/repos/octocat/Hello-World/compare/{base}...{head}", + "contents_url": "https://api.github.com/repos/octocat/Hello-World/contents/{+path}", + "contributors_url": "https://api.github.com/repos/octocat/Hello-World/contributors", + "deployments_url": "https://api.github.com/repos/octocat/Hello-World/deployments", + "downloads_url": "https://api.github.com/repos/octocat/Hello-World/downloads", + "events_url": "https://api.github.com/repos/octocat/Hello-World/events", + "forks_url": "https://api.github.com/repos/octocat/Hello-World/forks", + "git_commits_url": "https://api.github.com/repos/octocat/Hello-World/git/commits{/sha}", + "git_refs_url": "https://api.github.com/repos/octocat/Hello-World/git/refs{/sha}", + "git_tags_url": "https://api.github.com/repos/octocat/Hello-World/git/tags{/sha}", + "git_url": "git:github.com/octocat/Hello-World.git", + "issue_comment_url": "https://api.github.com/repos/octocat/Hello-World/issues/comments{/number}", + "issue_events_url": "https://api.github.com/repos/octocat/Hello-World/issues/events{/number}", + "issues_url": "https://api.github.com/repos/octocat/Hello-World/issues{/number}", + "keys_url": "https://api.github.com/repos/octocat/Hello-World/keys{/key_id}", + "labels_url": "https://api.github.com/repos/octocat/Hello-World/labels{/name}", + "languages_url": "https://api.github.com/repos/octocat/Hello-World/languages", + "merges_url": "https://api.github.com/repos/octocat/Hello-World/merges", + "milestones_url": "https://api.github.com/repos/octocat/Hello-World/milestones{/number}", + "notifications_url": "https://api.github.com/repos/octocat/Hello-World/notifications{?since,all,participating}", + "pulls_url": "https://api.github.com/repos/octocat/Hello-World/pulls{/number}", + "releases_url": "https://api.github.com/repos/octocat/Hello-World/releases{/id}", + "ssh_url": "git@github.com:octocat/Hello-World.git", + "stargazers_url": "https://api.github.com/repos/octocat/Hello-World/stargazers", + "statuses_url": "https://api.github.com/repos/octocat/Hello-World/statuses/{sha}", + "subscribers_url": "https://api.github.com/repos/octocat/Hello-World/subscribers", + "subscription_url": "https://api.github.com/repos/octocat/Hello-World/subscription", + "tags_url": "https://api.github.com/repos/octocat/Hello-World/tags", + "teams_url": "https://api.github.com/repos/octocat/Hello-World/teams", + "trees_url": "https://api.github.com/repos/octocat/Hello-World/git/trees{/sha}", + "clone_url": "https://github.com/octocat/Hello-World.git", + "mirror_url": "git:git.example.com/octocat/Hello-World", + "hooks_url": "https://api.github.com/repos/octocat/Hello-World/hooks", + "svn_url": "https://svn.github.com/octocat/Hello-World", + "homepage": "https://github.com", + "language": null, + "forks_count": 9, + "stargazers_count": 80, + "watchers_count": 80, + "size": 108, + "default_branch": "master", + "open_issues_count": 0, + "is_template": true, + "topics": [ + "octocat", + "atom", + "electron", + "api" + ], + "has_issues": true, + "has_projects": true, + "has_wiki": true, + "has_pages": false, + "has_downloads": true, + "archived": false, + "disabled": false, + "visibility": "public", + "pushed_at": "2011-01-26T19:06:43Z", + "created_at": "2011-01-26T19:01:12Z", + "updated_at": "2011-01-26T19:14:43Z", + "permissions": { + "admin": false, + "push": false, + "pull": true + }, + "allow_rebase_merge": true, + "temp_clone_token": "ABTLWHOULUVAXGTRYU7OC2876QJ2O", + "allow_squash_merge": true, + "allow_auto_merge": false, + "delete_branch_on_merge": true, + "allow_merge_commit": true, + "subscribers_count": 42, + "network_count": 0, + "license": { + "key": "mit", + "name": "MIT License", + "url": "https://api.github.com/licenses/mit", + "spdx_id": "MIT", + "node_id": "MDc6TGljZW5zZW1pdA==", + "html_url": "https://api.github.com/licenses/mit" + }, + "forks": 1, + "open_issues": 1, + "watchers": 1 + }, + "source": { + "id": 1296269, + "node_id": "MDEwOlJlcG9zaXRvcnkxMjk2MjY5", + "name": "Hello-World", + "full_name": "octocat/Hello-World", + "owner": { + "login": "octocat", + "id": 1, + "node_id": "MDQ6VXNlcjE=", + "avatar_url": "https://github.com/images/error/octocat_happy.gif", + "gravatar_id": "", + "url": "https://api.github.com/users/octocat", + "html_url": "https://github.com/octocat", + "followers_url": "https://api.github.com/users/octocat/followers", + "following_url": "https://api.github.com/users/octocat/following{/other_user}", + "gists_url": "https://api.github.com/users/octocat/gists{/gist_id}", + "starred_url": "https://api.github.com/users/octocat/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/octocat/subscriptions", + "organizations_url": "https://api.github.com/users/octocat/orgs", + "repos_url": "https://api.github.com/users/octocat/repos", + "events_url": "https://api.github.com/users/octocat/events{/privacy}", + "received_events_url": "https://api.github.com/users/octocat/received_events", + "type": "User", + "site_admin": false + }, + "private": false, + "html_url": "https://github.com/octocat/Hello-World", + "description": "This your first repo!", + "fork": false, + "url": "https://api.github.com/repos/octocat/Hello-World", + "archive_url": "https://api.github.com/repos/octocat/Hello-World/{archive_format}{/ref}", + "assignees_url": "https://api.github.com/repos/octocat/Hello-World/assignees{/user}", + "blobs_url": "https://api.github.com/repos/octocat/Hello-World/git/blobs{/sha}", + "branches_url": "https://api.github.com/repos/octocat/Hello-World/branches{/branch}", + "collaborators_url": "https://api.github.com/repos/octocat/Hello-World/collaborators{/collaborator}", + "comments_url": "https://api.github.com/repos/octocat/Hello-World/comments{/number}", + "commits_url": "https://api.github.com/repos/octocat/Hello-World/commits{/sha}", + "compare_url": "https://api.github.com/repos/octocat/Hello-World/compare/{base}...{head}", + "contents_url": "https://api.github.com/repos/octocat/Hello-World/contents/{+path}", + "contributors_url": "https://api.github.com/repos/octocat/Hello-World/contributors", + "deployments_url": "https://api.github.com/repos/octocat/Hello-World/deployments", + "downloads_url": "https://api.github.com/repos/octocat/Hello-World/downloads", + "events_url": "https://api.github.com/repos/octocat/Hello-World/events", + "forks_url": "https://api.github.com/repos/octocat/Hello-World/forks", + "git_commits_url": "https://api.github.com/repos/octocat/Hello-World/git/commits{/sha}", + "git_refs_url": "https://api.github.com/repos/octocat/Hello-World/git/refs{/sha}", + "git_tags_url": "https://api.github.com/repos/octocat/Hello-World/git/tags{/sha}", + "git_url": "git:github.com/octocat/Hello-World.git", + "issue_comment_url": "https://api.github.com/repos/octocat/Hello-World/issues/comments{/number}", + "issue_events_url": "https://api.github.com/repos/octocat/Hello-World/issues/events{/number}", + "issues_url": "https://api.github.com/repos/octocat/Hello-World/issues{/number}", + "keys_url": "https://api.github.com/repos/octocat/Hello-World/keys{/key_id}", + "labels_url": "https://api.github.com/repos/octocat/Hello-World/labels{/name}", + "languages_url": "https://api.github.com/repos/octocat/Hello-World/languages", + "merges_url": "https://api.github.com/repos/octocat/Hello-World/merges", + "milestones_url": "https://api.github.com/repos/octocat/Hello-World/milestones{/number}", + "notifications_url": "https://api.github.com/repos/octocat/Hello-World/notifications{?since,all,participating}", + "pulls_url": "https://api.github.com/repos/octocat/Hello-World/pulls{/number}", + "releases_url": "https://api.github.com/repos/octocat/Hello-World/releases{/id}", + "ssh_url": "git@github.com:octocat/Hello-World.git", + "stargazers_url": "https://api.github.com/repos/octocat/Hello-World/stargazers", + "statuses_url": "https://api.github.com/repos/octocat/Hello-World/statuses/{sha}", + "subscribers_url": "https://api.github.com/repos/octocat/Hello-World/subscribers", + "subscription_url": "https://api.github.com/repos/octocat/Hello-World/subscription", + "tags_url": "https://api.github.com/repos/octocat/Hello-World/tags", + "teams_url": "https://api.github.com/repos/octocat/Hello-World/teams", + "trees_url": "https://api.github.com/repos/octocat/Hello-World/git/trees{/sha}", + "clone_url": "https://github.com/octocat/Hello-World.git", + "mirror_url": "git:git.example.com/octocat/Hello-World", + "hooks_url": "https://api.github.com/repos/octocat/Hello-World/hooks", + "svn_url": "https://svn.github.com/octocat/Hello-World", + "homepage": "https://github.com", + "forks_count": 9, + "stargazers_count": 80, + "watchers_count": 80, + "size": 108, + "default_branch": "master", + "open_issues_count": 0, + "is_template": true, + "topics": [ + "octocat", + "atom", + "electron", + "api" + ], + "has_issues": true, + "has_projects": true, + "has_wiki": true, + "has_pages": false, + "has_downloads": true, + "archived": false, + "disabled": false, + "visibility": "public", + "pushed_at": "2011-01-26T19:06:43Z", + "created_at": "2011-01-26T19:01:12Z", + "updated_at": "2011-01-26T19:14:43Z", + "permissions": { + "admin": false, + "push": false, + "pull": true + }, + "allow_rebase_merge": true, + "temp_clone_token": "ABTLWHOULUVAXGTRYU7OC2876QJ2O", + "allow_squash_merge": true, + "allow_auto_merge": false, + "delete_branch_on_merge": true, + "allow_merge_commit": true, + "subscribers_count": 42, + "network_count": 0, + "license": { + "key": "mit", + "name": "MIT License", + "url": "https://api.github.com/licenses/mit", + "spdx_id": "MIT", + "node_id": "MDc6TGljZW5zZW1pdA==", + "html_url": "https://api.github.com/licenses/mit" + }, + "forks": 1, + "open_issues": 1, + "watchers": 1, + "security_and_analysis": { + "advanced_security": { + "status": "enabled" + }, + "secret_scanning": { + "status": "enabled" + }, + "secret_scanning_push_protection": { + "status": "disabled" + }, + "secret_scanning_non_provider_patterns": { + "status": "disabled" + } + } + } +} diff --git a/test/fixtures/settings.yml b/test/fixtures/settings.yml new file mode 100644 index 00000000..3a808505 --- /dev/null +++ b/test/fixtures/settings.yml @@ -0,0 +1,195 @@ +# This settings file can be used to create org-level settings + +# This is the settings that need to be applied to all repositories in the org +# See https://docs.github.com/en/rest/repos/repos?apiVersion=2022-11-28#create-an-organization-repository for all available settings for a repository +repository: + # A short description of the repository that will show up on GitHub + description: description of the repo + + # A URL with more information about the repository + homepage: https://example.github.io/ + + # Create an initial commit with empty README. + # Keep this set to true in most cases since many of the policies below cannot be implemented on bare repos + auto_init: true + + # The default branch for this repository. + default_branch: main + +# Rulesets +# See https://docs.github.com/en/rest/orgs/rules?apiVersion=2022-11-28#create-an-organization-repository-rulesetfor available options +rulesets: + - name: Template + # The target of the ruleset. Can be one of: + # - branch + # - tag + target: branch + # The enforcement level of the ruleset. `evaluate` allows admins to test + # rules before enforcing them. + # - disabled + # - active + # - evaluate + enforcement: active + + # The actors that can bypass the rules in this ruleset + bypass_actors: + - actor_id: number + # type: The type of actor that can bypass a ruleset + # - RepositoryRole + # - Team + # - Integration + # - OrganizationAdmin + actor_type: Team + # When the specified actor can bypass the ruleset. `pull_request` + # means that an actor can only bypass rules on pull requests. + # - always + # - pull_request + bypass_mode: pull_request + + - actor_id: 1 + actor_type: OrganizationAdmin + bypass_mode: always + + - actor_id: 7898 + actor_type: RepositoryRole + bypass_mode: always + + - actor_id: 210920 + actor_type: Integration + bypass_mode: always + + conditions: + # Parameters for a repository ruleset ref name condition + ref_name: + # Array of ref names or patterns to include. One of these + # patterns must match for the condition to pass. Also accepts + # `~DEFAULT_BRANCH` to include the default branch or `~ALL` to + # include all branches. + include: ["~DEFAULT_BRANCH"] + + # Array of ref names or patterns to exclude. The condition + # will not pass if any of these patterns match. + exclude: ["refs/heads/oldmaster"] + + # This condition only exists at the org level (remove for suborg and repo level rulesets) + repository_name: + # Array of repository names or patterns to include. + # One of these patterns must match for the condition + # to pass. Also accepts `~ALL` to include all + # repositories. + include: ["test*"] + # Array of repository names or patterns to exclude. The + # condition will not pass if any of these patterns + # match. + exclude: ["test", "test1"] + # Whether renaming of target repositories is + # prevented. + protected: true + + # Refer to https://docs.github.com/en/rest/orgs/rules#create-an-organization-repository-ruleset + rules: + - type: creation + - type: update + parameters: + # Branch can pull changes from its upstream repository + update_allows_fetch_and_merge: true + - type: deletion + - type: required_linear_history + - type: required_signatures + + - type: required_deployments + parameters: + required_deployment_environments: ["staging"] + + - type: pull_request + parameters: + # Reviewable commits pushed will dismiss previous pull + # request review approvals. + dismiss_stale_reviews_on_push: true + # Require an approving review in pull requests that modify + # files that have a designated code owner + require_code_owner_review: true + # Whether the most recent reviewable push must be approved + # by someone other than the person who pushed it. + require_last_push_approval: true + # The number of approving reviews that are required before a + # pull request can be merged. + required_approving_review_count: 1 + # All conversations on code must be resolved before a pull + # request can be merged. + required_review_thread_resolution: true + + # Choose which status checks must pass before branches can be merged + # into a branch that matches this rule. When enabled, commits must + # first be pushed to another branch, then merged or pushed directly + # to a branch that matches this rule after status checks have + # passed. + - type: required_status_checks + parameters: + # Whether pull requests targeting a matching branch must be + # tested with the latest code. This setting will not take + # effect unless at least one status check is enabled. + strict_required_status_checks_policy: true + required_status_checks: + - context: CodeQL + integration_id: 1234 + - context: GHAS Compliance + integration_id: 1234 + + # Choose which workflows must pass before branches can be merged. + - type: workflows + parameters: + workflows: + - path: .github/workflows/example.yml + # Run $("meta[name=octolytics-dimension-repository_id]").getAttribute('content') + # in the browser console of the repository to get the repository_id + repository_id: 123456 + # One of the following: + # Branch or tag + ref: refs/heads/main + # Commit SHA + sha: 1234567890abcdef + + - type: commit_message_pattern + parameters: + name: test commit_message_pattern + # required: + # - operator + # - pattern + negate: true + operator: starts_with + # The operator to use for matching. + # - starts_with + # - ends_with + # - contains + # - regex + pattern: skip* + # The pattern to match with. + + - type: commit_author_email_pattern + parameters: + name: test commit_author_email_pattern + negate: false + operator: regex + pattern: "^.*@example.com$" + + - type: committer_email_pattern + parameters: + name: test committer_email_pattern + negate: false + operator: regex + pattern: "^.*@example.com$" + + - type: branch_name_pattern + parameters: + name: test branch_name_pattern + negate: false + operator: regex + pattern: ".*\/.*" + + - type: "tag_name_pattern" + parameters: + name: test tag_name_pattern + negate: false + operator: regex + pattern: ".*\/.*" diff --git a/test/unit/index.test.js b/test/unit/index.test.js index 99982205..1cbfe13c 100644 --- a/test/unit/index.test.js +++ b/test/unit/index.test.js @@ -1,14 +1,29 @@ const { Probot } = require('probot') const { getLog } = require('probot/lib/helpers/get-log') const plugin = require('../../index') -/** @import { ProbotOctokit } from "probot" */ - -describe.skip('plugin', () => { - let app, event, sync +const env = require('../../lib/env') +const fs = require('fs') +const path = require('path') +/** @import { Probot, ProbotOctokit } from "probot" */ + +function getFixture (...paths) { + const pathToFixtures = path.resolve(__dirname, '..', 'fixtures', ...paths) + return fs.readFileSync(pathToFixtures, 'utf-8') +} +function getApiResponse (filename) { + const pathToFixtures = path.resolve(__dirname, '..', 'fixtures', 'api_responses', filename) + return { data: JSON.parse(fs.readFileSync(pathToFixtures, 'utf-8')) } +} + +describe('webhooks', () => { + /** @type {Probot} */ + let app + let event /** @type {InstanceType} */ let github - beforeEach(() => { + beforeEach(async () => { + /** @type {InstanceType} */ class Octokit { static defaults () { return Octokit @@ -18,8 +33,60 @@ describe.skip('plugin', () => { this.config = { get: jest.fn().mockReturnValue({}) } + this.paginate = jest.fn() + .mockImplementation(async (params) => { + if (params === 'GET /installation/repositories') { + return [{ owner: { login: 'bkeepers-inc' }, name: 'botland' }] + } else if (params && params.route === 'GET /orgs/{org}/rulesets') { + return [{ id: 21 }] + } else if (params && params.route === 'GET /orgs/{org}/rulesets/{id}') { + return [] + } else if (params && params.route === 'GET /orgs/{org}/installations') { + return [{ id: 21 }] + } else { + console.log({ params }) + throw new Error('not implemented') + } + }) this.repos = { - getContent: jest.fn(() => Promise.resolve({ data: { content: '' } })) + getContents: jest.fn().mockImplementation(() => Promise.resolve({ data: { content: '' } })), + getContent: jest.fn().mockImplementation((params) => { + if (params && params.path === '.github/settings.yml') { + return Promise.resolve({ data: { content: btoa(getFixture('settings.yml')) } }) + } else if (params && params.path === '.github') { + return Promise.resolve({ data: [{ name: 'repos', path: '.github/repos', sha: '2a97853ea484cd71a00e2cfe0dac45067b05b3e4' }] }) + } else if (params && params.path === '.github/suborgs') { + return Promise.resolve({ data: [] }) // Should return a list of files in that folder. GitHub would return 404 on an empty (non-existing) folder, but this works for testing purposes + } else { + return Promise.resolve({ data: { content: '' } }) // Usually we don't need any return value when this is called while testing + } + }), + get: jest.fn().mockResolvedValue(getApiResponse('get_repository.json')), + listCommits: jest.fn().mockResolvedValue({ data: { commits: { data: [{ sha: 'bb8a050117521bc7a01c2f691d5709da0510a387' }] } } }), + getBranch: jest.fn().mockResolvedValue({ data: { name: 'main' } }), + update: jest.fn().mockImplementation(() => null) + } + this.git = { + getTree: jest.fn().mockResolvedValue({ data: { tree: [{ path: 'botland.yml' }] } }) + } + this.request = Object.assign( + async function (route, parameters) { + if (route === 'POST /orgs/{org}/rulesets') { + return { url: 'mock call' } + } else { + console.log({ route, parameters }) + throw new Error('not implemented') + } + }, + { + endpoint: { merge: jest.fn().mockImplementation((route, options) => ({ route, options })) } + } + ) + this.apps = { + listInstallations: { + endpoint: { merge: jest.fn().mockImplementation(() => ({ route: 'GET /orgs/{org}/installations' })) } + }, + getAuthenticated: jest.fn().mockResolvedValue({ data: { slug: 'octoapp' } }) } } @@ -29,25 +96,20 @@ describe.skip('plugin', () => { } app = new Probot({ secret: 'abcdef', Octokit, log: getLog({ level: 'info' }) }) - github = { - repos: { - getContents: jest.fn(() => Promise.resolve({ data: { content: '' } })) - } - } - app.auth = () => Promise.resolve(github) + github = await app.auth() event = { name: 'push', payload: JSON.parse(JSON.stringify(require('../fixtures/events/push.settings.json'))) } - sync = jest.fn() + env.ADMIN_REPO = 'botland' - plugin(app, {}, { sync, FILE_NAME: '.github/settings.yml' }) + plugin(app, {}) }) describe('with settings modified on master', () => { it('syncs settings', async () => { await app.receive(event) - expect(sync).toHaveBeenCalled() + expect(github.repos.update).toHaveBeenCalled() }) }) @@ -58,7 +120,7 @@ describe.skip('plugin', () => { it('does not sync settings', async () => { await app.receive(event) - expect(sync).not.toHaveBeenCalled() + expect(github.repos.update).not.toHaveBeenCalled() }) }) @@ -69,7 +131,7 @@ describe.skip('plugin', () => { it('does not sync settings', async () => { await app.receive(event) - expect(sync).not.toHaveBeenCalled() + expect(github.repos.update).not.toHaveBeenCalled() }) }) @@ -83,7 +145,7 @@ describe.skip('plugin', () => { it('does sync settings', async () => { await app.receive(event) - expect(sync).toHaveBeenCalled() + expect(github.repos.update).toHaveBeenCalled() }) }) @@ -97,7 +159,7 @@ describe.skip('plugin', () => { it('does sync settings', async () => { await app.receive(event) - expect(sync).toHaveBeenCalled() + expect(github.repos.update).toHaveBeenCalled() }) }) @@ -111,7 +173,7 @@ describe.skip('plugin', () => { it('does sync settings', async () => { await app.receive(event) - expect(sync).toHaveBeenCalled() + expect(github.repos.update).toHaveBeenCalled() }) }) @@ -125,7 +187,7 @@ describe.skip('plugin', () => { it('does sync settings', async () => { await app.receive(event) - expect(sync).toHaveBeenCalled() + expect(github.repos.update).toHaveBeenCalled() }) }) @@ -139,7 +201,7 @@ describe.skip('plugin', () => { it('does sync settings', async () => { await app.receive(event) - expect(sync).toHaveBeenCalled() + expect(github.repos.update).toHaveBeenCalled() }) }) @@ -151,7 +213,7 @@ describe.skip('plugin', () => { it('does sync settings', async () => { await app.receive(event) - expect(sync).toHaveBeenCalled() + expect(github.repos.update).toHaveBeenCalled() }) }) From 585c7dab8bfd1a3c335032c3ba81a665ad96919f Mon Sep 17 00:00:00 2001 From: Torgeir S <80778551+Gramatus@users.noreply.github.com> Date: Sun, 15 Sep 2024 19:31:17 +0000 Subject: [PATCH 11/22] use probot logger in branches.test.js --- test/unit/lib/plugins/branches.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/unit/lib/plugins/branches.test.js b/test/unit/lib/plugins/branches.test.js index 68290ea4..63b17f3f 100644 --- a/test/unit/lib/plugins/branches.test.js +++ b/test/unit/lib/plugins/branches.test.js @@ -1,13 +1,13 @@ /* eslint-disable no-undef */ +const { getLog } = require('probot/lib/helpers/get-log') const { when } = require('jest-when') const Branches = require('../../../../lib/plugins/branches') describe('Branches', () => { let github - const log = jest.fn() - log.debug = jest.fn() - log.error = jest.fn() + const log = getLog() + log.level = 'debug' function configure (config) { const noop = false From 8bc68863efa45124752eff553568c57958d25105 Mon Sep 17 00:00:00 2001 From: Torgeir S <80778551+Gramatus@users.noreply.github.com> Date: Sun, 15 Sep 2024 19:30:53 +0000 Subject: [PATCH 12/22] return data when updating branch protection This is needed for the test to be able to check that it was called. --- lib/plugins/branches.js | 10 +++++++++- test/unit/lib/plugins/branches.test.js | 2 +- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/plugins/branches.js b/lib/plugins/branches.js index cac0aadf..30ab607a 100644 --- a/lib/plugins/branches.js +++ b/lib/plugins/branches.js @@ -73,7 +73,15 @@ module.exports = class Branches extends ErrorStash { return Promise.resolve(resArray) } this.log.debug(`Adding branch protection ${JSON.stringify(params)}`) - return this.github.repos.updateBranchProtection(params).then(res => this.log(`Branch protection applied successfully ${JSON.stringify(res.url)}`)).catch(e => { this.logError(`Error applying branch protection ${JSON.stringify(e)}`); return [] }) + return this.github.repos.updateBranchProtection(params) + .then(res => { + this.log.info(`Branch protection applied successfully ${JSON.stringify(res.url)}`) + return res.url + }) + .catch(e => { + this.logError(`Error applying branch protection ${JSON.stringify(e)}`) + return [] + }) }).catch((e) => { if (e.status === 404) { Object.assign(params, branch.protection, { headers: previewHeaders }) diff --git a/test/unit/lib/plugins/branches.test.js b/test/unit/lib/plugins/branches.test.js index 63b17f3f..42964997 100644 --- a/test/unit/lib/plugins/branches.test.js +++ b/test/unit/lib/plugins/branches.test.js @@ -7,7 +7,7 @@ const Branches = require('../../../../lib/plugins/branches') describe('Branches', () => { let github const log = getLog() - log.level = 'debug' + log.level = process.env.LOG_LEVEL ?? 'info' function configure (config) { const noop = false From 489546bab442dbc006fa579b8e164f9a54dcd732 Mon Sep 17 00:00:00 2001 From: Torgeir S <80778551+Gramatus@users.noreply.github.com> Date: Sun, 15 Sep 2024 19:33:04 +0000 Subject: [PATCH 13/22] fix broken branch test --- test/unit/lib/plugins/branches.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/lib/plugins/branches.test.js b/test/unit/lib/plugins/branches.test.js index 42964997..8b6e49d1 100644 --- a/test/unit/lib/plugins/branches.test.js +++ b/test/unit/lib/plugins/branches.test.js @@ -28,7 +28,7 @@ describe('Branches', () => { enforce_admins: { enabled: false } } }), - updateBranchProtection: jest.fn().mockImplementation(() => Promise.resolve('updateBranchProtection')), + updateBranchProtection: jest.fn().mockImplementation(() => Promise.resolve({ url: 'updateBranchProtection' })), deleteBranchProtection: jest.fn().mockImplementation(() => Promise.resolve('deleteBranchProtection')) } } @@ -204,7 +204,7 @@ describe('Branches', () => { }) }) - describe.skip('return values', () => { + describe('return values', () => { it('returns updateBranchProtection Promise', () => { const plugin = configure( [{ From 0bb27133cec6aaa89b71c7258d51812cf06c5eef Mon Sep 17 00:00:00 2001 From: Torgeir S <80778551+Gramatus@users.noreply.github.com> Date: Sun, 15 Sep 2024 20:17:38 +0000 Subject: [PATCH 14/22] use probot logger in milestones.test.js --- test/unit/lib/plugins/milestones.test.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/unit/lib/plugins/milestones.test.js b/test/unit/lib/plugins/milestones.test.js index 93cde7db..24061fee 100644 --- a/test/unit/lib/plugins/milestones.test.js +++ b/test/unit/lib/plugins/milestones.test.js @@ -1,8 +1,11 @@ +const { getLog } = require('probot/lib/helpers/get-log') const Milestones = require('../../../../lib/plugins/milestones') describe.skip('Milestones', () => { let github + const log = getLog() + log.level = process.env.LOG_LEVEL ?? 'info' function configure (config) { return new Milestones(github, { owner: 'bkeepers', repo: 'test' }, config) } From 5178080e29b424d9af12cb04bca51886c3ea31ed Mon Sep 17 00:00:00 2001 From: Torgeir S <80778551+Gramatus@users.noreply.github.com> Date: Sun, 15 Sep 2024 20:19:32 +0000 Subject: [PATCH 15/22] include `title` in `NAME_FIELDS` Also adds support for ovveriding the filter list in `Diffable.sync`. This is needed for milestones to work correctly. However, it should be tested well to ensure it does not break anything else. --- lib/mergeDeep.js | 2 +- lib/plugins/diffable.js | 5 +++-- lib/plugins/milestones.js | 5 +++-- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/mergeDeep.js b/lib/mergeDeep.js index 054a5b34..e15b1b58 100644 --- a/lib/mergeDeep.js +++ b/lib/mergeDeep.js @@ -1,7 +1,7 @@ const mergeBy = require('./mergeArrayBy') const DeploymentConfig = require('./deploymentConfig') -const NAME_FIELDS = ['name', 'username', 'actor_id', 'login', 'type', 'key_prefix'] +const NAME_FIELDS = ['name', 'username', 'actor_id', 'login', 'type', 'key_prefix', 'title'] const NAME_USERNAME_PROPERTY = item => NAME_FIELDS.find(prop => Object.prototype.hasOwnProperty.call(item, prop)) const GET_NAME_USERNAME_PROPERTY = item => { if (NAME_USERNAME_PROPERTY(item)) return item[NAME_USERNAME_PROPERTY(item)] } diff --git a/lib/plugins/diffable.js b/lib/plugins/diffable.js index 8db6a0ef..ac1c6ba9 100644 --- a/lib/plugins/diffable.js +++ b/lib/plugins/diffable.js @@ -24,13 +24,14 @@ const MergeDeep = require('../mergeDeep') const NopCommand = require('../nopcommand') const ignorableFields = ['id', 'node_id', 'default', 'url'] module.exports = class Diffable extends ErrorStash { - constructor (nop, github, repo, entries, log, errors) { + constructor (nop, github, repo, entries, log, errors, filterNameFields) { super(errors) this.github = github this.repo = repo this.entries = entries this.log = log this.nop = nop + this.filterNameFields = filterNameFields ?? MergeDeep.NAME_FIELDS } filterEntries () { @@ -102,7 +103,7 @@ module.exports = class Diffable extends ErrorStash { } } // Delete any diffable that now only has name and no other attributes - filteredEntries = filteredEntries.filter(entry => Object.keys(entry).filter(key => !MergeDeep.NAME_FIELDS.includes(key)).length !== 0) + filteredEntries = filteredEntries.filter(entry => Object.keys(entry).filter(key => !this.filterNameFields.includes(key)).length !== 0) const changes = [] diff --git a/lib/plugins/milestones.js b/lib/plugins/milestones.js index 9fc93ab2..bfbc8b53 100644 --- a/lib/plugins/milestones.js +++ b/lib/plugins/milestones.js @@ -1,8 +1,9 @@ +const MergeDeep = require('../mergeDeep') const Diffable = require('./diffable') module.exports = class Milestones extends Diffable { - constructor (...args) { - super(...args) + constructor (nop, github, repo, entries, log, errors) { + super(nop, github, repo, entries, log, errors, MergeDeep.NAME_FIELDS.filter(i => i !== 'title')) if (this.entries) { this.entries.forEach(milestone => { From 16925d685fe25b7854e3b9bdf2e240f2f4f37e13 Mon Sep 17 00:00:00 2001 From: Torgeir S <80778551+Gramatus@users.noreply.github.com> Date: Sun, 15 Sep 2024 20:19:55 +0000 Subject: [PATCH 16/22] fix broken milestones test --- test/unit/lib/plugins/milestones.test.js | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/test/unit/lib/plugins/milestones.test.js b/test/unit/lib/plugins/milestones.test.js index 24061fee..c08bab38 100644 --- a/test/unit/lib/plugins/milestones.test.js +++ b/test/unit/lib/plugins/milestones.test.js @@ -1,22 +1,24 @@ const { getLog } = require('probot/lib/helpers/get-log') const Milestones = require('../../../../lib/plugins/milestones') -describe.skip('Milestones', () => { +describe('Milestones', () => { let github const log = getLog() log.level = process.env.LOG_LEVEL ?? 'info' function configure (config) { - return new Milestones(github, { owner: 'bkeepers', repo: 'test' }, config) + const noop = false + const errors = [] + return new Milestones(noop, github, { owner: 'bkeepers', repo: 'test' }, config, log, errors) } beforeEach(() => { github = { - paginate: jest.fn().mockImplementation(() => Promise.resolve()), + paginate: jest.fn().mockResolvedValue({}), issues: { - listMilestonesForRepo: { + listMilestones: { endpoint: { - merge: jest.fn().mockImplementation(() => {}) + merge: jest.fn().mockImplementation(() => ({ route: 'GET /repos/{owner}/{repo}/milestones' })) } }, createMilestone: jest.fn().mockImplementation(() => Promise.resolve()), @@ -28,12 +30,12 @@ describe.skip('Milestones', () => { describe('sync', () => { it('syncs milestones', async () => { - github.paginate.mockReturnValueOnce(Promise.resolve([ + github.paginate.mockResolvedValueOnce([ { title: 'no-change', description: 'no-change-description', due_on: null, state: 'open', number: 5 }, { title: 'new-description', description: 'old-description', due_on: null, state: 'open', number: 2 }, { title: 'new-state', description: 'FF0000', due_on: null, state: 'open', number: 4 }, { title: 'remove-milestone', description: 'old-description', due_on: null, state: 'open', number: 1 } - ])) + ]) const plugin = configure([ { title: 'no-change', description: 'no-change-description', due_on: '2019-03-29T07:00:00Z', state: 'open' }, From 996af693c37df92a7069f7afbc9f80ac61653dc5 Mon Sep 17 00:00:00 2001 From: Torgeir S Date: Sat, 5 Oct 2024 16:32:40 +0200 Subject: [PATCH 17/22] refact: changed to async/await and removed some returns I believe this change makes the logic easier to follow, especially not returning data that does not need to be returned. --- lib/settings.js | 47 +++++++++++++++++++---------------------------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/lib/settings.js b/lib/settings.js index f05c413d..09acfb4d 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -307,9 +307,8 @@ ${this.results.reduce((x, y) => { const rulesetsConfig = this.config.rulesets if (rulesetsConfig) { const RulesetsPlugin = Settings.PLUGINS.rulesets - return new RulesetsPlugin(this.nop, this.github, this.repo, rulesetsConfig, this.log, this.errors, SCOPE.ORG).sync().then(res => { - this.appendToResults(res) - }) + const res = await new RulesetsPlugin(this.nop, this.github, this.repo, rulesetsConfig, this.log, this.errors, SCOPE.ORG).sync() + this.appendToResults(res) } } @@ -349,15 +348,12 @@ ${this.results.reduce((x, y) => { this.log.debug(`found a matching repoconfig for this repo ${JSON.stringify(repoConfig)}`) const childPlugins = this.childPluginsList(repo) const RepoPlugin = Settings.PLUGINS.repository - return new RepoPlugin(this.nop, this.github, repo, repoConfig, this.installation_id, this.log, this.errors).sync().then(res => { - this.appendToResults(res) - return Promise.all( - childPlugins.map(([Plugin, config]) => { - return new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync() - })) - }).then(res => { + const res = await new RepoPlugin(this.nop, this.github, repo, repoConfig, this.installation_id, this.log, this.errors).sync(); + this.appendToResults(res) + await Promise.all(childPlugins.map(async ([Plugin, config]) => { + const res = await new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync() this.appendToResults(res) - }) + })) } catch (e) { if (this.nop) { const nopcommand = new NopCommand(this.constructor.name, this.repo, null, `${e}`, 'ERROR') @@ -371,10 +367,9 @@ ${this.results.reduce((x, y) => { } else { this.log.debug(`Didnt find any a matching repoconfig for this repo ${JSON.stringify(repo)} in ${JSON.stringify(this.repoConfigs)}`) const childPlugins = this.childPluginsList(repo) - return Promise.all(childPlugins.map(([Plugin, config]) => { - return new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync().then(res => { - this.appendToResults(res) - }) + await Promise.all(childPlugins.map(async ([Plugin, config]) => { + const res = await new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync() + this.appendToResults(res) })) } } @@ -382,9 +377,7 @@ ${this.results.reduce((x, y) => { async updateAll() { // this.subOrgConfigs = this.subOrgConfigs || await this.getSubOrgConfigs(this.github, this.repo, this.log) // this.repoConfigs = this.repoConfigs || await this.getRepoConfigs(this.github, this.repo, this.log) - return this.eachRepositoryRepos(this.github, this.config.restrictedRepos, this.log).then(res => { - this.appendToResults(res) - }) + await this.eachRepositoryRepos(this.github, this.config.restrictedRepos, this.log) } getSubOrgConfig(repoName) { @@ -495,17 +488,15 @@ ${this.results.reduce((x, y) => { async eachRepositoryRepos(github, restrictedRepos, log) { log.debug('Fetching repositories') - return github.paginate('GET /installation/repositories').then(repositories => { - return Promise.all(repositories.map(repository => { - if (this.isRestricted(repository.name)) { - return null - } + const repositories = await github.paginate('GET /installation/repositories') + await Promise.all(repositories.map(async repository => { + if (this.isRestricted(repository.name)) { + return + } - const { owner, name } = repository - return this.updateRepos({ owner: owner.login, repo: name }) - }) - ) - }) + const { owner, name } = repository + await this.updateRepos({ owner: owner.login, repo: name }) + })) } /** From cb401ec5f207945b7927fffbdf7f50239f15f0c4 Mon Sep 17 00:00:00 2001 From: Torgeir S Date: Sat, 5 Oct 2024 16:38:05 +0200 Subject: [PATCH 18/22] refact: avoid repeating code in `updateRepos` This change should give the exact same result, except moving the `else`-block within the try/catch (and I don't see why that is bad). The refactor makes it easier to understand the difference in logic with and without a present `repoConfig` by only having the actual alternate action within the if/else and keeping the bit that always happen outside the if/else. --- lib/settings.js | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/lib/settings.js b/lib/settings.js index 09acfb4d..5f755efa 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -343,34 +343,30 @@ ${this.results.reduce((x, y) => { if (overrideRepoConfig) { repoConfig = this.mergeDeep.mergeDeep({}, repoConfig, overrideRepoConfig) } - if (repoConfig) { - try { + try { + const childPlugins = this.childPluginsList(repo) + if (repoConfig) { this.log.debug(`found a matching repoconfig for this repo ${JSON.stringify(repoConfig)}`) - const childPlugins = this.childPluginsList(repo) const RepoPlugin = Settings.PLUGINS.repository const res = await new RepoPlugin(this.nop, this.github, repo, repoConfig, this.installation_id, this.log, this.errors).sync(); this.appendToResults(res) - await Promise.all(childPlugins.map(async ([Plugin, config]) => { - const res = await new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync() - this.appendToResults(res) - })) - } catch (e) { - if (this.nop) { - const nopcommand = new NopCommand(this.constructor.name, this.repo, null, `${e}`, 'ERROR') - this.log.error(`NOPCOMMAND ${JSON.stringify(nopcommand)}`) - this.appendToResults([nopcommand]) - // throw e - } else { - throw e - } } - } else { - this.log.debug(`Didnt find any a matching repoconfig for this repo ${JSON.stringify(repo)} in ${JSON.stringify(this.repoConfigs)}`) - const childPlugins = this.childPluginsList(repo) + else { + this.log.debug(`Didnt find any a matching repoconfig for this repo ${JSON.stringify(repo)} in ${JSON.stringify(this.repoConfigs)}`) + } await Promise.all(childPlugins.map(async ([Plugin, config]) => { const res = await new Plugin(this.nop, this.github, repo, config, this.log, this.errors).sync() this.appendToResults(res) })) + } catch (e) { + if (this.nop) { + const nopcommand = new NopCommand(this.constructor.name, this.repo, null, `${e}`, 'ERROR') + this.log.error(`NOPCOMMAND ${JSON.stringify(nopcommand)}`) + this.appendToResults([nopcommand]) + // throw e + } else { + throw e + } } } From cac9b842f482c4e64379b695858529745ca78836 Mon Sep 17 00:00:00 2001 From: Torgeir S Date: Sat, 5 Oct 2024 17:01:59 +0200 Subject: [PATCH 19/22] FORK_ONLY: fix some errors found by ts checker --- index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index fdffe6b9..7154fc8b 100644 --- a/index.js +++ b/index.js @@ -14,7 +14,7 @@ let deploymentConfig * @param {Probot} robot * @param {ApplicationFunctionOptions} probotOptions */ -module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => { +module.exports = (robot, _, Settings = require('./lib/settings')) => { let appName = 'safe-settings' let appSlug = 'safe-settings' async function syncAllSettings (nop, context, repo = context.repo(), ref) { @@ -209,7 +209,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => }) } - async function createCheckRun (context, pull_request, head_sha, head_branch) { + async function createCheckRun (context, pull_request, head_sha) { const { payload } = context // robot.log.debug(`Check suite was requested! for ${context.repo()} ${pull_request.number} ${head_sha} ${head_branch}`) const res = await context.octokit.checks.create({ @@ -233,7 +233,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => const app = await github.apps.getAuthenticated() appName = app.data.name appSlug = app.data.slug - robot.log.debug(`Validated the app is configured properly = \n${JSON.stringify(app.data, null, 2)}`) + robot.log.debug(`Validated the app ${appName} is configured properly = \n${JSON.stringify(app.data, null, 2)}`) } } @@ -443,7 +443,7 @@ module.exports = (robot, { getRouter }, Settings = require('./lib/settings')) => } catch (error) { if (error.status === 404) { // if the a config file does not exist, create one from the old one - const update = await context.octokit.request('PUT /repos/{owner}/{repo}/contents/{path}', { + await context.octokit.request('PUT /repos/{owner}/{repo}/contents/{path}', { owner: payload.repository.owner.login, repo: env.ADMIN_REPO, path: newPath, From 9b0c66b97d08c936277872c80693049c00f12ba4 Mon Sep 17 00:00:00 2001 From: Torgeir S Date: Sat, 5 Oct 2024 17:16:53 +0200 Subject: [PATCH 20/22] chore: autoformatted according to rules after #664 --- index.js | 34 +++++++++++++++++----------------- lib/settings.js | 8 ++++---- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/index.js b/index.js index 7154fc8b..408425bb 100644 --- a/index.js +++ b/index.js @@ -17,7 +17,7 @@ let deploymentConfig module.exports = (robot, _, Settings = require('./lib/settings')) => { let appName = 'safe-settings' let appSlug = 'safe-settings' - async function syncAllSettings (nop, context, repo = context.repo(), ref) { + async function syncAllSettings(nop, context, repo = context.repo(), ref) { try { deploymentConfig = await loadYamlFileSystem() robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`) @@ -46,7 +46,7 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { } } - async function syncSubOrgSettings (nop, context, suborg, repo = context.repo(), ref) { + async function syncSubOrgSettings(nop, context, suborg, repo = context.repo(), ref) { try { deploymentConfig = await loadYamlFileSystem() robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`) @@ -71,7 +71,7 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { } } - async function syncSettings (nop, context, repo = context.repo(), ref) { + async function syncSettings(nop, context, repo = context.repo(), ref) { try { deploymentConfig = await loadYamlFileSystem() robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`) @@ -96,7 +96,7 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { } } - async function renameSync (nop, context, repo = context.repo(), rename, ref) { + async function renameSync(nop, context, repo = context.repo(), rename, ref) { try { deploymentConfig = await loadYamlFileSystem() robot.log.debug(`deploymentConfig is ${JSON.stringify(deploymentConfig)}`) @@ -105,7 +105,7 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { const config = Object.assign({}, deploymentConfig, runtimeConfig) const renameConfig = Object.assign({}, config, rename) robot.log.debug(`config for ref ${ref} is ${JSON.stringify(config)}`) - return Settings.sync(nop, context, repo, renameConfig, ref ) + return Settings.sync(nop, context, repo, renameConfig, ref) } catch (e) { if (nop) { let filename = env.SETTINGS_FILE_PATH @@ -127,7 +127,7 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { * * @return The parsed YAML file */ - async function loadYamlFileSystem () { + async function loadYamlFileSystem() { if (deploymentConfig === undefined) { const deploymentConfigPath = env.DEPLOYMENT_CONFIG_FILE if (fs.existsSync(deploymentConfigPath)) { @@ -139,7 +139,7 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { return deploymentConfig } - function getAllChangedSubOrgConfigs (payload) { + function getAllChangedSubOrgConfigs(payload) { const settingPattern = new Glob(`${env.CONFIG_PATH}/suborgs/*.yml`) // Changes will be an array of files that were added const added = payload.commits.map(c => { @@ -163,7 +163,7 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { return configs } - function getAllChangedRepoConfigs (payload, owner) { + function getAllChangedRepoConfigs(payload, owner) { const settingPattern = new Glob(`${env.CONFIG_PATH}/repos/*.yml`) // Changes will be an array of files that were added const added = payload.commits.map(c => { @@ -186,7 +186,7 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { return configs } - function getChangedRepoConfigName (glob, files, owner) { + function getChangedRepoConfigName(glob, files, owner) { const modifiedFiles = files.filter(s => { robot.log.debug(JSON.stringify(s)) return (s.search(glob) >= 0) @@ -197,7 +197,7 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { }) } - function getChangedSubOrgConfigName (glob, files) { + function getChangedSubOrgConfigName(glob, files) { const modifiedFiles = files.filter(s => { robot.log.debug(JSON.stringify(s)) return (s.search(glob) >= 0) @@ -209,7 +209,7 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { }) } - async function createCheckRun (context, pull_request, head_sha) { + async function createCheckRun(context, pull_request, head_sha) { const { payload } = context // robot.log.debug(`Check suite was requested! for ${context.repo()} ${pull_request.number} ${head_sha} ${head_branch}`) const res = await context.octokit.checks.create({ @@ -238,7 +238,7 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { } - async function syncInstallation () { + async function syncInstallation() { robot.log.trace('Fetching installations') const github = await robot.auth() @@ -399,7 +399,7 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { }) robot.on('repository.renamed', async context => { - if (env.BLOCK_REPO_RENAME_BY_HUMAN!== 'true') { + if (env.BLOCK_REPO_RENAME_BY_HUMAN !== 'true') { robot.log.debug(`"env.BLOCK_REPO_RENAME_BY_HUMAN" is 'false' by default. Repo rename is not managed by Safe-settings. Continue with the default behavior.`) return } @@ -418,7 +418,7 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { const newPath = `.github/repos/${payload.repository.name}.yml` robot.log.debug(oldPath) try { - const repofile = await context.octokit.request('GET /repos/{owner}/{repo}/contents/{path}', { + const repofile = await context.octokit.request('GET /repos/{owner}/{repo}/contents/{path}', { owner: payload.repository.owner.login, repo: env.ADMIN_REPO, path: oldPath, @@ -447,7 +447,7 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { owner: payload.repository.owner.login, repo: env.ADMIN_REPO, path: newPath, - name: `${payload.repository.name}.yml`, + name: `${payload.repository.name}.yml`, content: content, message: `Repo Renamed and safe-settings renamed the file from ${payload.changes.repository.name.from} to ${payload.repository.name}`, sha: repofile.data.sha, @@ -472,8 +472,8 @@ module.exports = (robot, _, Settings = require('./lib/settings')) => { } else { robot.log.debug('Repository Edited by a Human') // Create a repository config to reset the name back to the previous name - const rename = {repository: { name: payload.changes.repository.name.from, oldname: payload.repository.name}} - const repo = {repo: payload.changes.repository.name.from, owner: payload.repository.owner.login} + const rename = { repository: { name: payload.changes.repository.name.from, oldname: payload.repository.name } } + const repo = { repo: payload.changes.repository.name.from, owner: payload.repository.owner.login } return renameSync(false, context, repo, rename) } }) diff --git a/lib/settings.js b/lib/settings.js index 5f755efa..fe04283f 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -17,7 +17,7 @@ class Settings { * @param {boolean} nop * @param {Context} context */ - static async syncAll (nop, context, repo, config, ref) { + static async syncAll(nop, context, repo, config, ref) { const settings = new Settings(nop, context, repo, config, ref) try { await settings.loadConfigs() @@ -183,10 +183,10 @@ class Settings { //remove duplicate rows in this.results this.results = this.results.filter((thing, index, self) => { - return index === self.findIndex((t) => { - return t.type === thing.type && t.repo === thing.repo && t.plugin === thing.plugin - }) + return index === self.findIndex((t) => { + return t.type === thing.type && t.repo === thing.repo && t.plugin === thing.plugin }) + }) let error = false // Different logic From 444cefe36e359fba0b1485f1edbd4f6dd7d7a55b Mon Sep 17 00:00:00 2001 From: Torgeir S <80778551+Gramatus@users.noreply.github.com> Date: Sun, 15 Sep 2024 10:43:33 +0000 Subject: [PATCH 21/22] fix: update eslint ecmaVersion to 13 Linting failed in deploymentConfig.js due to the static fields. Eslint supports this from ecmaVersion 13. --- .eslintrc.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.eslintrc.json b/.eslintrc.json index 5db37214..84dd1b73 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -8,7 +8,7 @@ "eslint:recommended" ], "parserOptions": { - "ecmaVersion": 12 + "ecmaVersion": 13 }, "rules": { }, From b4ea065c1d9da58e8b5f86185ae65fbc552ea194 Mon Sep 17 00:00:00 2001 From: Torgeir S <80778551+Gramatus@users.noreply.github.com> Date: Sun, 15 Sep 2024 10:45:30 +0000 Subject: [PATCH 22/22] reapply eslint to tests, still supporting jest The full test suite failed because it tried to lint the tests, while the running linter did not do the same. Enabling linting gave errors due to jest having undef stuff, but overriding these files for jest env keeps the linting without these errors. --- .eslintrc.json | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/.eslintrc.json b/.eslintrc.json index 84dd1b73..7c9e3f26 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -12,5 +12,12 @@ }, "rules": { }, - "ignorePatterns": ["test/**/*.js"] + "overrides": [ + { + "files": ["test/**/*.js"], + "env": { + "jest": true + } + } + ] }