From 6c6b348d89ec721617271de94f24126e07adc126 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Mon, 3 Apr 2023 14:44:42 +0200 Subject: [PATCH 01/18] Add review files --- spec/checkNewCronJobSpec.js | 520 ------------------ spec/checkPullRequestJobSpec.js | 931 -------------------------------- 2 files changed, 1451 deletions(-) delete mode 100644 spec/checkNewCronJobSpec.js delete mode 100644 spec/checkPullRequestJobSpec.js diff --git a/spec/checkNewCronJobSpec.js b/spec/checkNewCronJobSpec.js deleted file mode 100644 index 6f7a4766..00000000 --- a/spec/checkNewCronJobSpec.js +++ /dev/null @@ -1,520 +0,0 @@ -// Copyright 2021 The Oppia Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS-IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - - -require('dotenv').config(); -const { createProbot } = require('probot'); -// The plugin refers to the actual app in index.js. -const oppiaBot = require('../index'); -const checkCronJobModule = require('../lib/checkNewCronJobs'); -const checkPullRequestJobModule = require('../lib/checkPullRequestJob'); -const apiForSheetsModule = require('../lib/apiForSheets'); -const checkPullRequestLabelsModule = require('../lib/checkPullRequestLabels'); -const checkPullRequestBranchModule = require('../lib/checkPullRequestBranch'); -const checkCriticalPullRequestModule = - require('../lib/checkCriticalPullRequest'); -const checkPullRequestTemplateModule = - require('../lib/checkPullRequestTemplate'); -const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); -const scheduler = require('../lib/scheduler'); -const { JOBS_AND_FETURES_TESTING_WIKI_LINK } = require('../lib/utils'); -let payloadData = JSON.parse( - JSON.stringify(require('../fixtures/pullRequestPayload.json')) -); - -describe('Cron Job Spec', () => { - /** - * @type {import('probot').Probot} robot - */ - let robot; - - /** - * @type {import('probot').Octokit} github - */ - let github; - - /** - * @type {import('probot').Application} app - */ - let app; - - const firstNewJobFileObj = { - sha: 'bbc41f1bd8b2c90c91a1335d149be7a132a732d8', - filename: 'core/controllers/cron.py', - status: 'added', - additions: 37, - deletions: 0, - changes: 37, - blob_url: - 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/cron.py', - raw_url: - 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/domain/cron.py', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/core/' + - 'cron.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', - patch: '@@ -0,0 +1,37 @@\n+class JobStatusMailerHandler(base.BaseHanr):' + - '\n+ """Handler for mailing admin about job failures."""\n+\n+' + - ' @acl_decorators.can_perform_cron_tasks\n+ def get(self):\n+' + - ' """Handles GET requests."""\n+ # TODO(sll): Get the 50 most' + - 'recent failed shards, not all of them.\n+ failed_jobs' + - ' = cron_services.get_stuck_jobs(TWENTY_FIVE_HOURS_IN_MSECS)\n+' + - ' if failed_jobs:\n+ email_subject = \'MapReduce failure' + - ' alert\'\n+ email_message = (\n+ \'%s jobs' + - ' have failed in the past 25 hours. More information \'\n+ ' + - ' \'(about at most %s jobs; to see more, please check the logs):\'\n+' + - ' ) % (len(failed_jobs), MAX_JOBS_TO_REPORT_ON)\n+\n+' + - ' for job in failed_jobs[:MAX_JOBS_TO_REPORT_ON]:\n+' + - ' email_message += \'\\n\'\n+ ' + - 'email_message += \'-----------------------------------\'\n+ ' + - ' email_message += \'\\n\'\n+ ' + - ' email_message += (\n+ \'Job with mapreduce ID %s' + - '(key name %s) failed. \'\n+ \'More info:\\n\\n\'\n+' - }; - - const urlJobFileObj = { - sha: 'f06a0d3ea104733080c4dad4a4e5aa7fb76d8f5d', - filename: 'main_cron.py', - status: 'modified', - additions: 43, - deletions: 0, - changes: 43, - blob_url: - 'https://github.com/oppia/oppia/blob/5b0e633fa1b9a00771a3b88302fa' + - '3ff048d7240c/main_cron.py', - raw_url: - 'https://github.com/oppia/oppia/raw/5b0e633fa1b9a00771a3b88302fa' + - '3ff048d7240c/main_cron.py', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/' + - 'main_cron.py?ref=5b0e633fa1b9a00771a3b88302fa3ff048d7240c', - patch: - '@@ -0,0 +1,46 @@\n+"""Main package for URL routing and the index' + - 'page."""\n+\n+from __future__ import absolute_import # pylint:' + - ' disable=import-only-modules\n+from __future__' + - ' import unicode_literals # pylint: disable=import-only-modules\n' + - '+\n+from core.controllers import cron\n+from core.platform import' + - ' n+\n+\n+transaction_services = models.Registry.' + - ' import_transaction_services()\n+\n+# Register the URLs with' + - 'the classes responsible for handling them.\n+URLS = [\n+ ' + - 'main.get_redirect_route(\n+ r\'/cron/mail/admin/job_s' + - 'tatus\', cron.JobStatusMailerHandler),\n+' + - 'main.get_redirect_route(\n+ ' + - 'r\'/cron/users/dashboard_stats\', cron.CronDashboardStatsHandler),\n+' + - ' main.get_redirect_route(\n+ r\'/cron/users/user_deletion' + - 'r\'/cron/users/fully_complete_user_deletion\',\n+' + - ' cron.CronFullyCompleteUserDeletionHandler),\n+' + - ' main.get_redirect_route(\n+tionRecommendationsHandler),\n+' + - 'main.get_redirect_route(\n+ r\'/cron/explorations/search_rank\',\n+' + - 'cron.CronActivitySearchRankHandler),\n+ main.get_redirect_route(\n' + - ' main.get_redirect_route(\n+ r\'/cron/models/cleanup\', cron' - }; - - const jobTestFile = { - sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', - filename: 'core/controllers/cron_test.py', - status: 'added', - additions: 1, - deletions: 0, - changes: 1, - blob_url: - 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/cpntrollers/cron_test.py', - raw_url: - 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/domain/cron_test.py', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/core/domain/' + - 'exp_jobs_oppiabot_off.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', - patch: - '@@ -0,0 +1,25 @@\n+class CronJobTests(test_utils.GenericTestBase):\n+' + - 'FIVE_WEEKS = datetime.timedelta(weeks=5)\n+ NINE_WEEKS' + - '= datetime.timedelta(weeks=9)\n+\n+ def setUp(self):\n+' + - ' super(CronJobTests, self).setUp()\n+ ' + - ' self.signup(self.ADMIN_EMAIL, self.ADMIN_USERNAME)\n+' + - 'self.admin_id = self.get_user_id_from_email(self.ADMIN_EMAIL)\n+' + - 'self.set_admins([self.ADMIN_USERNAME])\n+' + - ' self.testapp_swap = self.swap(\n+' + - 'self, \'testapp\', webtest.TestApp(main_cron.app))\n+\n+' + - 'self.email_subjects = []\n+ self.email_bodies = []\n+' + - 'def _mock_send_mail_to_admin(email_subject, email_body):\n+ ' + - 'ocks email_manager.send_mail_to_admin() as it\'s not possible to\n+' + - 'send mail with self.testapp_swap, i.e with the URLs defined in\n+' + - ' main_cron.\n+ """\n+ self.email_subjects' + - '.append(email_subject)\n+ self.email_bodies.append' + - '(email_body)\n+\n+ self.send_mail_to_admin_swap = self.swap' + - '(\n+ email_manager, \'send_mail_to_admin\',' + - '_mock_send_mail_to_admin)\n' - }; - - const nonJobFile = { - sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', - filename: 'core/domain/exp_fetchers.py', - status: 'added', - additions: 1, - deletions: 0, - changes: 1, - blob_url: - 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/domain/exp_fetchers.py', - raw_url: - 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/domain/exp_fetchers.py', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/core/domain/' + - 'exp_fetchers.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', - patch: '@@ -0,0 +1 @@\n+# def _migrate_states_schema' + - '(versioned_exploration_states, exploration_id):', - }; - - beforeEach(() => { - spyOn(scheduler, 'createScheduler').and.callFake(() => { }); - - github = { - issues: { - createComment: jasmine.createSpy('createComment').and.returnValue({}), - addLabels: jasmine.createSpy('addLabels').and.returnValue({}), - addAssignees: jasmine.createSpy('addAssignees').and.returnValue({}) - }, - }; - - robot = createProbot({ - id: 1, - cert: 'test', - githubToken: 'test', - }); - - app = robot.load(oppiaBot); - spyOn(app, 'auth').and.resolveTo(github); - spyOn(checkCronJobModule, 'checkForNewCronJob').and.callThrough(); - spyOn( - checkPullRequestJobModule, 'checkForNewJob').and.callFake(() => { }); - spyOn(apiForSheetsModule, 'checkClaStatus').and.callFake(() => { }); - spyOn( - checkCriticalPullRequestModule, 'checkIfPRAffectsDatastoreLayer' - ).and.callFake(() => { }); - spyOn(checkPullRequestBranchModule, 'checkBranch').and.callFake(() => { }); - spyOn( - checkPullRequestTemplateModule, 'checkTemplate' - ).and.callFake(() => { }); - spyOn(newCodeOwnerModule, 'checkForNewCodeowner').and.callFake(() => { }); - }); - - describe('When a new cron job is added in a pull request', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - nonJobFile, firstNewJobFileObj - ], - }), - }; - payloadData.payload.pull_request.changed_files = 2; - await robot.receive(payloadData); - }); - - it('should check for cron jobs', () => { - expect(checkCronJobModule.checkForNewCronJob).toHaveBeenCalled(); - }); - - it('should get modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - it('should ping server jobs admin', () => { - expect(github.issues.createComment).toHaveBeenCalled(); - const author = payloadData.payload.pull_request.user.login; - const formText = ( - 'server jobs form'.link( - 'https://goo.gl/forms/XIj00RJ2h5L55XzU2') - ); - const newLineFeed = '
'; - const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); - - expect(github.issues.createComment).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - body: - 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, ' + - 'it adds a new cron job.' + newLineFeed + 'Also @' + author + - ' please add the new test and URL redirects for the new CRON jobs ' + - 'It seems you have added or edited a CRON job, if so please request ' + - 'a testing of this CRON job with this ' + formText + ' Please refer ' + - 'to ' + wikiLinkText + ' for details.' + newLineFeed + 'Thanks!', - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - }); - }); - - it('should assign server jobs admin', () => { - expect(github.issues.addAssignees).toHaveBeenCalled(); - expect(github.issues.addAssignees).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - assignees: ['vojtechjelinek', 'DubeySandeep', 'kevintab95'] - }); - }); - - it('should add datastore label', () => { - expect(github.issues.addLabels).toHaveBeenCalled(); - expect(github.issues.addLabels).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - labels: ['PR: Affects datastore layer'] - }); - }); - }); - - describe('When a new cron job is added in an existing cron job file', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - urlJobFileObj, jobTestFile, firstNewJobFileObj - ], - }), - }; - payloadData.payload.pull_request.changed_files = 1; - await robot.receive(payloadData); - }); - - it('should check for cron jobs', () => { - expect(checkCronJobModule.checkForNewCronJob).toHaveBeenCalled(); - }); - - it('should get modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - - it('should ping server jobs admin', () => { - expect(github.issues.createComment).toHaveBeenCalled(); - const author = payloadData.payload.pull_request.user.login; - const formText = ( - 'server jobs form'.link('https://goo.gl/forms/XIj00RJ2h5L55XzU2') - ); - const newLineFeed = '
'; - const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); - - expect(github.issues.createComment).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - body: - 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, it ' + - 'adds a new cron job.' + newLineFeed + 'Also @' + author + ' It seems' + - ' you have added or edited a CRON job, if so please request a testing' + - ' of this CRON job with this ' + formText + ' Please refer to ' + - wikiLinkText + ' for details.' + newLineFeed + 'Thanks!', - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - }); - }); - - it('should assign server jobs admin', () => { - expect(github.issues.addAssignees).toHaveBeenCalled(); - expect(github.issues.addAssignees).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - assignees: ['vojtechjelinek', 'DubeySandeep', 'kevintab95'] - }); - }); - - it('should add datastore label', () => { - expect(github.issues.addLabels).toHaveBeenCalled(); - expect(github.issues.addLabels).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - labels: ['PR: Affects datastore layer'] - }); - }); - }); - - describe('When no job file is modified in a pull request', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - nonJobFile - ], - }), - }; - - payloadData.payload.pull_request.changed_files = 1; - await robot.receive(payloadData); - }); - - it('should check for cron jobs', () => { - expect(checkCronJobModule.checkForNewCronJob).toHaveBeenCalled(); - }); - - it('should not get modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - it('should not ping server job admin', () => { - expect(github.issues.createComment).not.toHaveBeenCalled(); - }); - - it('should not add datastore label', () => { - expect(github.issues.addLabels).not.toHaveBeenCalled(); - }); - }); - - describe('When test and URL redirects are added for a cron job', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - jobTestFile, urlJobFileObj - ], - }), - }; - - payloadData.payload.pull_request.changed_files = 1; - await robot.receive(payloadData); - }); - - it('should check for cron jobs', () => { - expect(checkCronJobModule.checkForNewCronJob).toHaveBeenCalled(); - }); - - it('should get modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - it('should ping server job admin', () => { - expect(github.issues.createComment).toHaveBeenCalled(); - const author = payloadData.payload.pull_request.user.login; - const formText = ( - 'server jobs form'.link('https://goo.gl/forms/XIj00RJ2h5L55XzU2') - ); - const newLineFeed = '
'; - const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); - - expect(github.issues.createComment).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - body: - 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, it ' + - 'adds a new cron job.' + newLineFeed + 'Also @' + author + ' It seems' + - ' you have added or edited a CRON job, if so please request a testing' + - ' of this CRON job with this ' + formText + ' Please refer to ' + - wikiLinkText + ' for details.' + newLineFeed + 'Thanks!', - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - }); - }); - - it('should add datastore label', () => { - expect(github.issues.addLabels).toHaveBeenCalled(); - expect(github.issues.addLabels).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - labels: ['PR: Affects datastore layer'] - }); - }); - }); - - describe('When only tests are added for a cron job', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - jobTestFile - ], - }), - }; - - payloadData.payload.pull_request.changed_files = 1; - await robot.receive(payloadData); - }); - - it('should check for cron jobs', () => { - expect(checkCronJobModule.checkForNewCronJob).toHaveBeenCalled(); - }); - - it('should get modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - it('should ping server job admin', () => { - expect(github.issues.createComment).toHaveBeenCalled(); - const author = payloadData.payload.pull_request.user.login; - const formText = ( - 'server jobs form'.link('https://goo.gl/forms/XIj00RJ2h5L55XzU2') - ); - const newLineFeed = '
'; - const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); - - expect(github.issues.createComment).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - body: - 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, it ' + - 'adds a new cron job.' + newLineFeed + 'Also @' + author + ' It seems' + - ' you have added or edited a CRON job, if so please request a testing' + - ' of this CRON job with this ' + formText + ' Please refer to ' + - wikiLinkText + ' for details.' + newLineFeed + 'Thanks!', - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - }); - }); - - it('should add datastore label', () => { - expect(github.issues.addLabels).toHaveBeenCalled(); - expect(github.issues.addLabels).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - labels: ['PR: Affects datastore layer'] - }); - }); - }); - - describe('When pull request has datastore label', () => { - beforeEach(async () => { - payloadData.payload.pull_request.labels = [{ - name: 'PR: Affects datastore layer' - }]; - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - nonJobFile, firstNewJobFileObj - ], - }), - }; - - payloadData.payload.pull_request.changed_files = 2; - await robot.receive(payloadData); - }); - - it('should check for cron jobs', () => { - expect(checkCronJobModule.checkForNewCronJob).toHaveBeenCalled(); - }); - - it('should not get modified files', () => { - expect(github.pulls.listFiles).not.toHaveBeenCalled(); - }); - - it('should not ping server job admin', () => { - expect(github.issues.createComment).not.toHaveBeenCalled(); - }); - }); -}); diff --git a/spec/checkPullRequestJobSpec.js b/spec/checkPullRequestJobSpec.js deleted file mode 100644 index 521f3868..00000000 --- a/spec/checkPullRequestJobSpec.js +++ /dev/null @@ -1,931 +0,0 @@ -// Copyright 2020 The Oppia Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS-IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -require('dotenv').config(); -const { createProbot } = require('probot'); -// The plugin refers to the actual app in index.js. -const oppiaBot = require('../index'); -const checkPullRequestJobModule = require('../lib/checkPullRequestJob'); -const apiForSheetsModule = require('../lib/apiForSheets'); -const checkPullRequestLabelsModule = require('../lib/checkPullRequestLabels'); -const checkPullRequestBranchModule = require('../lib/checkPullRequestBranch'); -const checkCriticalPullRequestModule = - require('../lib/checkCriticalPullRequest'); -const checkPullRequestTemplateModule = - require('../lib/checkPullRequestTemplate'); -const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); -const scheduler = require('../lib/scheduler'); -const checkCronJobModule = require('../lib/checkNewCronJobs'); -const { JOBS_AND_FETURES_TESTING_WIKI_LINK } = require('../lib/utils'); - -let payloadData = JSON.parse( - JSON.stringify(require('../fixtures/pullRequestPayload.json')) -); - -describe('Pull Request Job Spec', () => { - /** - * @type {import('probot').Probot} robot - */ - let robot; - - /** - * @type {import('probot').Octokit} github - */ - let github; - - /** - * @type {import('probot').Application} app - */ - let app; - - const firstNewJobFileObj = { - sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', - filename: 'core/jobs/exp_jobs_one_off.py', - status: 'added', - additions: 1, - deletions: 0, - changes: 1, - blob_url: - 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/jobs/exp_jobs_one_off.py', - raw_url: - 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/jobs/exp_jobs_one_off.py', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/core/jobs/' + - 'exp_jobs_one_off.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', - patch: '@@ -0,0 +1 @@\n+class FirstTestJob(jobs.JobTestBase):\n' + - '+ """One-off job for creating and populating ' + - 'UserContributionsModels for\n+ ' + - 'all registered users that have contributed.\n+ """\n+ ' + - '@classmethod\n+ def entity_classes_to_map_over(cls)' + - ':\n+ """Return a list of datastore class references ' + - 'to map over."""\n+ return [exp_models.' + - 'ExplorationSnapshotMetadataModel]\n+\n+ @staticmethod\n+ ' + - 'def map(item):\n+ """Implements the map function for this ' + - 'job."""\n+ yield (\n+ item.committer_id, ' + - '{\n+ \'exploration_id\': item.' + - 'get_unversioned_instance_id(),\n+ ' + - '\'version_string\': item.get_version_string(),\n+ ' + - '})\n+\n+\n+ @staticmethod\n+ def reduce(key, ' + - 'version_and_exp_ids):\n+ """Implements the reduce ' + - 'function for this job."""\n+ created_exploration_ids = ' + - 'set()\n+ edited_exploration_ids = set()\n+\n+ ' + - 'edits = [ast.literal_eval(v) for v in version_and_exp_ids]' + - '\n+\n+ for edit in edits:\n+ ' + - 'edited_exploration_ids.add(edit[\'exploration_id\'])' + - '\n+ if edit[\'version_string\'] == \'1\'' + - ':\n+ created_exploration_ids.add(edit' + - '[\'exploration_id\'])\n+\n+ if user_services.' + - 'get_user_contributions(key, strict=False) is not None' + - ':\n+ user_services.update_user_contributions' + - '(\n+ key, list(created_exploration_ids), ' + - 'list(\n+ edited_exploration_ids))\n+ ' + - 'else:\n+ user_services.create_user_contributions' + - '(\n+ key, list(created_exploration_ids), list' + - '(\n+ edited_exploration_ids))\n', - }; - - const secondNewJobFileObj = { - sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', - filename: 'core/jobs/exp_jobs_oppiabot_off.py', - status: 'added', - additions: 1, - deletions: 0, - changes: 1, - blob_url: - 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', - raw_url: - 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/core/jobs/' + - 'exp_jobs_oppiabot_off.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', - patch: '@@ -0,0 +1 @@\n+class SecondTestJob(jobs.JobTestBase):\n' + - '+ """One-off job for creating and populating ' + - 'UserContributionsModels for\n' + - '+ all registered users that have contributed.\n+ ' + - '"""\n+ @classmethod\n+ def entity_classes_to_map_over' + - '(cls):\n+ """Return a list of datastore class references ' + - 'to map over."""\n+ return [exp_models.' + - 'ExplorationSnapshotMetadataModel]\n+\n+ @staticmethod\n+ ' + - 'def map(item):\n+ """Implements the map function for this ' + - 'job."""\n+ yield (\n+ item.committer_id, ' + - '{\n+ \'exploration_id\': item.' + - 'get_unversioned_instance_id(),\n+ \'version_string\'' + - ': item.get_version_string(),\n+ })\n+\n+\n+ ' + - '@staticmethod\n+ def reduce(key, version_and_exp_ids)' + - ':\n+ """Implements the reduce function for this job.' + - '"""\n+ created_exploration_ids = set()\n+ ' + - 'edited_exploration_ids = set()\n+\n+ edits = ' + - '[ast.literal_eval(v) for v in version_and_exp_ids]\n+\n+ ' + - 'for edit in edits:\n+ edited_exploration_ids.add' + - '(edit[\'exploration_id\'])\n+ if edit' + - '[\'version_string\'] == \'1\':\n+ ' + - 'created_exploration_ids.add(edit[\'exploration_id\'])' + - '\n+\n+ if user_services.get_user_contributions' + - '(key, strict=False) is not None:\n+ user_services.' + - 'update_user_contributions(\n+ key, list' + - '(created_exploration_ids), list(\n+ ' + - 'edited_exploration_ids))\n+ else:\n+ ' + - 'user_services.create_user_contributions(\n+ ' + - 'key, list(created_exploration_ids), list(\n+ ' + - 'edited_exploration_ids))\n', - }; - - const modifiedExistingJobFileObj = { - sha: 'f06a0d3ea104733080c4dad4a4e5aa7fb76d8f5d', - filename: 'core/jobs/user_jobs_one_off.py', - status: 'modified', - additions: 43, - deletions: 0, - changes: 43, - blob_url: - 'https://github.com/oppia/oppia/blob/5b0e633fa1b9a00771a3b88302fa' + - '3ff048d7240c/core/jobs/user_jobs_one_off.py', - raw_url: - 'https://github.com/oppia/oppia/raw/5b0e633fa1b9a00771a3b88302fa' + - '3ff048d7240c/core/jobs/user_jobs_one_off.py', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/core/jobs/' + - 'user_jobs_one_off.py?ref=5b0e633fa1b9a00771a3b88302fa3ff048d7240c', - patch: - '@@ -80,6 +80,49 @@ def reduce(key, version_and_exp_ids)' + - ':\n edited_exploration_ids))\n \n ' + - '\n+class OppiabotContributionsJob(jobs.JobTestBase):\n' + - '+ """One-off job for creating and populating ' + - 'UserContributionsModels for' + - '\n+ all registered users that have contributed.\n+ ' + - '"""\n+ @classmethod\n+ def entity_classes_to_map_over' + - '(cls):\n+ """Return a list of datastore class ' + - 'references to map over."""\n+ return ' + - '[exp_models.ExplorationSnapshotMetadataModel]\n+\n+ ' + - '@staticmethod\n+ def map(item):\n+ """Implements ' + - 'the map function for this job."""\n+ yield ' + - '(\n+ item.committer_id, {\n+ ' + - '\'exploration_id\': item.get_unversioned_instance_id(),' + - '\n+ \'version_string\': item.' + - 'get_version_string(),\n+ })\n+\n+\n+ ' + - '@staticmethod\n+ def reduce(key, version_and_exp_ids)' + - ':\n+ """Implements the reduce function for this ' + - 'job."""\n+ created_exploration_ids = set()' + - '\n+ edited_exploration_ids = set()\n+\n+ ' + - 'edits = [ast.literal_eval(v) for v in version_and_exp_ids]' + - '\n+\n+ for edit in edits:\n+ ' + - 'edited_exploration_ids.add(edit[\'exploration_id\'])' + - '\n+ if edit[\'version_string\'] == \'1\'' + - ':\n+ created_exploration_ids.add(edit' + - '[\'exploration_id\'])\n+\n+ if user_services.' + - 'get_user_contributions(key, strict=False) is not None' + - ':\n+ user_services.update_user_contributions' + - '(\n+ key, list(created_exploration_ids), ' + - 'list(\n+ edited_exploration_ids))' + - '\n+ else:\n+ user_services.' + - 'create_user_contributions(\n+ key, ' + - 'list(created_exploration_ids), list(' + - '\n+ edited_exploration_ids))\n', - }; - const modifiedExistingJobFileObjWithJobClassInPatch = { - sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', - filename: 'core/jobs/exp_jobs_oppiabot_off.py', - status: 'modified', - additions: 1, - deletions: 0, - changes: 1, - blob_url: - 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', - raw_url: - 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/core/jobs/' + - 'exp_jobs_oppiabot_off.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', - patch: '@@ -0,0 +1 @@class SecondTestJob(jobs.JobTestBase) ' + - 'def reduce(key, version_and_exp_ids):\n' + - ' edited_exploration_ids))\n ', - }; - - const jobFileObjWithJobClassInPatchAndNewJob = { - sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', - filename: 'core/jobs/exp_jobs_oppiabot_off.py', - status: 'modified', - additions: 1, - deletions: 0, - changes: 1, - blob_url: - 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', - raw_url: - 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a89413' + - '0e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/core/jobs/' + - 'exp_jobs_oppiabot_off.py?ref=' + - '67fb4a973b318882af3b5a894130e110d7e9833c', - patch: '@@ -0,0 +1 @@class SecondTestJob(jobs.JobTestBase) ' + - 'def reduce(key, version_and_exp_ids):\n' + - ' edited_exploration_ids))\n' + - ' \n' + - '+class AnotherTestJob(jobs.JobTestBase):\n' + - '+ """\n' + - '+ @classmethod\n' + - '+ def entity_classes_to_map_over ', - }; - - const jobFromTestDir = { - sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', - filename: 'core/tests/linter_tests/exp_jobs_oppiabot_off.py', - status: 'added', - additions: 1, - deletions: 0, - changes: 1, - blob_url: - 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/tests/linter_tests/exp_jobs_oppiabot_off.py', - raw_url: - 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/tests/linter_tests/exp_jobs_oppiabot_off.py', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/core/tests/' + - 'linter_tests/exp_jobs_oppiabot_off.py?ref=' + - '67fb4a973b318882af3b5a894130e110d7e9833c', - patch: '@@ -0,0 +1 @@\n+class LintTestJob(jobs.JobTestBase):\n' + - '+ """One-off job for creating and populating' + - ' UserContributionsModels for\n' + - '+ all registered users that have contributed.\n+ ' + - '"""\n+ @classmethod\n+ def entity_classes_to_map_over' + - '(cls):\n+ """Return a list of datastore class ' + - 'references to map over."""\n+ return [exp_models.' + - 'ExplorationSnapshotMetadataModel]\n+\n+ @staticmethod' + - '\n+ def map(item):\n+ """Implements the map function ' + - 'for this job."""\n+ yield (\n+ item.' + - 'committer_id, {\n+ \'exploration_id\': ' + - 'item.get_unversioned_instance_id(),\n+ ' + - '\'version_string\': item.get_version_string(),\n+ ' + - '})\n+\n+\n+ @staticmethod\n+ def reduce(key, ' + - 'version_and_exp_ids):\n+ """Implements the reduce ' + - 'function for this job."""\n+ created_exploration_ids ' + - '= set()\n+ edited_exploration_ids = set()\n+\n+ ' + - 'edits = [ast.literal_eval(v) for v in version_and_exp_ids]' + - '\n+\n+ for edit in edits:\n+ ' + - 'edited_exploration_ids.add(edit[\'exploration_id\'])' + - '\n+ if edit[\'version_string\'] == \'1\'' + - ':\n+ created_exploration_ids.add(edit' + - '[\'exploration_id\'])\n+\n+ if user_services.' + - 'get_user_contributions(key, strict=False) is not None' + - ':\n+ user_services.update_user_contributions' + - '(\n+ key, list(created_exploration_ids), ' + - 'list(\n+ edited_exploration_ids))\n+ ' + - 'else:\n+ user_services.create_user_contributions' + - '(\n+ key, list(created_exploration_ids), list' + - '(\n+ edited_exploration_ids))\n', - }; - - const fileWithMultipleJobs = { - sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', - filename: 'core/jobs/exp_jobs_oppiabot_off.py', - status: 'added', - additions: 1, - deletions: 0, - changes: 1, - blob_url: - 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', - raw_url: - 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/core/jobs/' + - 'exp_jobs_oppiabot_off.py?ref=' + - '67fb4a973b318882af3b5a894130e110d7e9833c', - patch: '@@ -0,0 +1 @@\n+class TestJob(jobs.JobTestBase):\n' + - '+ """One-off job for creating and ' + - 'populating UserContributionsModels for \n+class ' + - 'AnotherTestJob(jobs.JobTestBase):\n' + - '+ """\n+ @classmethod\n+ def ' + - 'entity_classes_to_map_over(cls):\n+ """Return a list' + - ' of datastore class references to map over."""\n+ ' + - 'return [exp_models.ExplorationSnapshotMetadataModel]\n+\n+ ' + - '@staticmethod\n+ def map(item):\n+ """Implements ' + - 'the map function for this job."""\n+ yield ' + - '(\n+ item.committer_id, {\n+ ' + - '\'exploration_id\': item.get_unversioned_instance_id(),\n+' + - ' \'version_string\': item.get_version_string(),' + - '\n+ })\n+\n+\n+ @staticmethod\n+ ' + - 'def reduce(key, version_and_exp_ids):\n+ ' + - '"""Implements the reduce function for this job."""\n+ ' + - 'created_exploration_ids = set()\n+ ' + - 'edited_exploration_ids = set()\n+\n+ edits = ' + - '[ast.literal_eval(v) for v in version_and_exp_ids]' + - '\n+\n+ for edit in edits:\n+ ' + - 'edited_exploration_ids.add(edit[\'exploration_id\'])' + - '\n+ if edit[\'version_string\'] == \'1\':\n+' + - ' created_exploration_ids.add(edit' + - '[\'exploration_id\'])\n+\n+ if user_services.' + - 'get_user_contributions(key, strict=False) is not None' + - ':\n+ user_services.update_user_contributions' + - '(\n+ key, list(created_exploration_ids), ' + - 'list(\n+ edited_exploration_ids))' + - '\n+ else:\n+ user_services.' + - 'create_user_contributions(\n+ key, list(' + - 'created_exploration_ids), list(\n+ ' + - 'edited_exploration_ids))\n', - }; - - const jobTestFile = { - sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', - filename: 'core/jobs/exp_jobs_oppiabot_off_test.py', - status: 'added', - additions: 1, - deletions: 0, - changes: 1, - blob_url: - 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', - raw_url: - 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/core/jobs/' + - 'exp_jobs_oppiabot_off.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', - patch: '@@ -0,0 +1 @@\n+class TestJobTests(jobs.JobTestBase):\n' + - '+ """One-off job for creating and populating ' + - 'UserContributionsModels for \n' + - '+class AnotherTestJobTests(jobs.JobTestBase):\n' + - '+ """\n+ ' + - '@classmethod\n+ def entity_classes_to_map_over(cls):' + - '\n+ """Return a list of datastore class references to ' + - 'map over."""\n+ return [exp_models.' + - 'ExplorationSnapshotMetadataModel]\n+\n+ @staticmethod\n+ ' + - 'def map(item):\n+ """Implements the map function for this ' + - 'job."""\n+ yield (\n+ item.committer_id, ' + - '{\n+ \'exploration_id\': item.get_unversioned_' + - 'instance_id(),\n+ \'version_string\': item.' + - 'get_version_string(),\n+ })\n+\n+\n+ ' + - '@staticmethod\n+ def reduce(key, version_and_exp_ids):\n+' + - ' """Implements the reduce function for this job."""\n+' + - ' created_exploration_ids = set()\n+ ' + - 'edited_exploration_ids = set()\n+\n+ edits ' + - '= [ast.literal_eval(v) for v in version_and_exp_ids]\n+\n+ ' + - 'for edit in edits:\n+ edited_exploration_ids.add(edit' + - '[\'exploration_id\'])\n+ if edit[\'version_string\'] == ' + - '\'1\':\n+ created_exploration_ids.add(edit' + - '[\'exploration_id\'])\n+\n+ if user_services.' + - 'get_user_contributions(key, strict=False) is not None:\n+ ' + - 'user_services.update_user_contributions(\n+ key, list' + - '(created_exploration_ids), list(\n+ ' + - 'edited_exploration_ids))\n+ else:\n+ ' + - 'user_services.create_user_contributions(\n+ ' + - 'key, list(created_exploration_ids), list(\n+ ' + - 'edited_exploration_ids))\n', - }; - - const nonJobFile = { - sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', - filename: 'core/domain/exp_fetchers.py', - status: 'added', - additions: 1, - deletions: 0, - changes: 1, - blob_url: - 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/domain/exp_fetchers.py', - raw_url: - 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/domain/exp_fetchers.py', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/core/domain/' + - 'exp_fetchers.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', - patch: '@@ -0,0 +1 @@\n+# def _migrate_states_schema' + - '(versioned_exploration_states, exploration_id):', - }; - - beforeEach(() => { - spyOn(scheduler, 'createScheduler').and.callFake(() => { }); - - github = { - issues: { - createComment: jasmine.createSpy('createComment').and.returnValue({}), - addLabels: jasmine.createSpy('addLabels').and.returnValue({}), - addAssignees: jasmine.createSpy('addAssignees').and.returnValue({}) - }, - }; - - robot = createProbot({ - id: 1, - cert: 'test', - githubToken: 'test', - }); - - app = robot.load(oppiaBot); - spyOn(app, 'auth').and.resolveTo(github); - spyOn(checkPullRequestJobModule, 'checkForNewJob').and.callThrough(); - spyOn(apiForSheetsModule, 'checkClaStatus').and.callFake(() => { }); - spyOn( - checkCriticalPullRequestModule, 'checkIfPRAffectsDatastoreLayer' - ).and.callFake(() => { }); - spyOn(checkPullRequestBranchModule, 'checkBranch').and.callFake(() => { }); - spyOn(checkCronJobModule, 'checkForNewCronJob').and.callFake(() => { }); - spyOn( - checkPullRequestTemplateModule, 'checkTemplate' - ).and.callFake(() => { }); - spyOn(newCodeOwnerModule, 'checkForNewCodeowner').and.callFake(() => { }); - }); - - describe('When a new job file is created in a pull request', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [nonJobFile, firstNewJobFileObj], - }), - }; - payloadData.payload.pull_request.changed_files = 2; - await robot.receive(payloadData); - }); - - it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); - }); - - it('should get modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - it('should ping server jobs admin', () => { - const author = payloadData.payload.pull_request.user.login; - const formText = ( - 'server jobs form'.link( - 'https://goo.gl/forms/XIj00RJ2h5L55XzU2') - ); - const newLineFeed = '
'; - const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); - const jobNameLink = ( - 'FirstTestJob'.link(firstNewJobFileObj.blob_url) - ); - - expect(github.issues.createComment).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - body: - 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, ' + - 'it adds a new job. The name of the job is ' + jobNameLink + '.' + - newLineFeed + - 'Also @' + author + ', please make sure to fill in the ' + formText + - ' for the new job to be tested on the backup server. ' + - 'This PR can be merged only after the test run is successful. ' + - 'Please refer to ' + wikiLinkText + ' for details.' + - newLineFeed + 'Thanks!', - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - }); - }); - - it('should assign server jobs admin', () => { - expect(github.issues.addAssignees).toHaveBeenCalled(); - expect(github.issues.addAssignees).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - assignees: ['vojtechjelinek', 'DubeySandeep', 'kevintab95'] - }); - }); - - it('should add datastore label', () => { - expect(github.issues.addLabels).toHaveBeenCalled(); - expect(github.issues.addLabels).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - labels: ['PR: Affects datastore layer'] - }); - }); - }); - - describe('When multiple job files are created in a pull request', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - nonJobFile, - firstNewJobFileObj, - secondNewJobFileObj - ], - }), - }; - - payloadData.payload.pull_request.changed_files = 3; - await robot.receive(payloadData); - }); - - it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); - }); - - it('should get modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - it('should ping server jobs admin', () => { - expect(github.issues.createComment).toHaveBeenCalled(); - const author = payloadData.payload.pull_request.user.login; - const formText = ( - 'server jobs form'.link('https://goo.gl/forms/XIj00RJ2h5L55XzU2') - ); - const newLineFeed = '
'; - const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); - const jobRegistryLink = ( - 'job registry'.link( - 'https://github.com/oppia/oppia/blob/develop/core/jobs_registry.py') - ); - const firstJobNameLink = ( - 'FirstTestJob'.link(firstNewJobFileObj.blob_url) - ); - const secondJobNameLink = ( - 'SecondTestJob'.link(secondNewJobFileObj.blob_url) - ); - expect(github.issues.createComment).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - body: - 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, ' + - 'it adds new jobs. The jobs are ' + firstJobNameLink + - ', ' + secondJobNameLink + '.' + newLineFeed + 'Also @' + - author + ', please make sure to fill in the ' + - formText + ' for the new jobs to be tested on the backup server. ' + - 'This PR can be merged only after the test run is successful. ' + - 'Please refer to ' + wikiLinkText + ' for details.' + - newLineFeed + 'Thanks!', - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - }); - }); - - it('should assign server jobs admin', () => { - expect(github.issues.addAssignees).toHaveBeenCalled(); - expect(github.issues.addAssignees).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - assignees: ['vojtechjelinek', 'DubeySandeep', 'kevintab95'] - }); - }); - - it('should add datastore label', () => { - expect(github.issues.addLabels).toHaveBeenCalled(); - expect(github.issues.addLabels).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - labels: ['PR: Affects datastore layer'] - }); - }); - }); - - describe('When a new job file is created and registry is ' + - 'updated in a pull request', - () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - nonJobFile, - firstNewJobFileObj - ], - }), - }; - payloadData.payload.pull_request.changed_files = 2; - await robot.receive(payloadData); - }); - - it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); - }); - - it('should get modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - it('should ping server jobs admin', () => { - expect(github.issues.createComment).toHaveBeenCalled(); - const author = payloadData.payload.pull_request.user.login; - const formText = ( - 'server jobs form'.link('https://goo.gl/forms/XIj00RJ2h5L55XzU2') - ); - const newLineFeed = '
'; - const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); - const jobNameLink = ( - 'FirstTestJob'.link(firstNewJobFileObj.blob_url) - ); - expect(github.issues.createComment).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - body: - 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this ' + - 'PR, it adds a new job. The name of the job is ' + jobNameLink + - '.' + newLineFeed + 'Also @' + author + - ', please make sure to fill in the ' + formText + - ' for the new job to be tested on the backup server. ' + - 'This PR can be merged only after the test run is successful. ' + - 'Please refer to ' + wikiLinkText + ' for details.' + - newLineFeed + 'Thanks!', - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - }); - }); - - it('should assign server jobs admin', () => { - expect(github.issues.addAssignees).toHaveBeenCalled(); - expect(github.issues.addAssignees).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - assignees: ['vojtechjelinek', 'DubeySandeep', 'kevintab95'] - }); - }); - - it('should add datastore label', () => { - expect(github.issues.addLabels).toHaveBeenCalled(); - expect(github.issues.addLabels).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - labels: ['PR: Affects datastore layer'] - }); - }); - } - ); - - describe('When a new job is added in an existing job file', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - modifiedExistingJobFileObj, - ], - }), - }; - payloadData.payload.pull_request.changed_files = 1; - await robot.receive(payloadData); - }); - - it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); - }); - - it('should get modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - - it('should ping server jobs admin', () => { - expect(github.issues.createComment).toHaveBeenCalled(); - const author = payloadData.payload.pull_request.user.login; - const formText = ( - 'server jobs form'.link('https://goo.gl/forms/XIj00RJ2h5L55XzU2') - ); - const newLineFeed = '
'; - const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); - const jobRegistryLink = ( - 'job registry'.link( - 'https://github.com/oppia/oppia/blob/develop/core/jobs_registry.py') - ); - const jobNameLink = ( - 'OppiabotContributionsJob' - .link(modifiedExistingJobFileObj.blob_url) - ); - expect(github.issues.createComment).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - body: - 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, ' + - 'it adds a new job. The name of the job is ' + jobNameLink + - '.' + newLineFeed + 'Also @' + author + ', please make sure to ' + - 'fill in the ' + formText + ' for the new job to be tested on the ' + - 'backup server. ' + 'This PR can be merged only after the test ' + - 'run is successful. ' + 'Please refer to ' + wikiLinkText + - ' for details.' + newLineFeed + 'Thanks!', - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - }); - }); - - it('should assign server jobs admin', () => { - expect(github.issues.addAssignees).toHaveBeenCalled(); - expect(github.issues.addAssignees).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - assignees: ['vojtechjelinek', 'DubeySandeep', 'kevintab95'] - }); - }); - - it('should add datastore label', () => { - expect(github.issues.addLabels).toHaveBeenCalled(); - expect(github.issues.addLabels).toHaveBeenCalledWith({ - issue_number: payloadData.payload.pull_request.number, - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - labels: ['PR: Affects datastore layer'] - }); - }); - }); - - describe('When an existing job file is modified with no new job', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - { - ...firstNewJobFileObj, - status: 'modified', - patch: '\n+# No job files present in the changes', - }, - ], - }), - }; - - payloadData.payload.pull_request.changed_files = 1; - await robot.receive(payloadData); - }); - - it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); - }); - - it('should get modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - - it('should not ping server jobs admin', () => { - expect(github.issues.createComment).not.toHaveBeenCalled(); - }); - it('should not add datastore label', () => { - expect(github.issues.addLabels).not.toHaveBeenCalled(); - }); - it('should not assign server jobs admin', () => { - expect(github.issues.addAssignees).not.toHaveBeenCalled(); - }); - }); - - describe('When no job file is modified in a pull request', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - nonJobFile - ], - }), - }; - - payloadData.payload.pull_request.changed_files = 1; - await robot.receive(payloadData); - }); - - it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); - }); - - it('should not get modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - it('should not ping server job admin', () => { - expect(github.issues.createComment).not.toHaveBeenCalled(); - }); - }); - - describe('Does not comment on job from test dir', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - jobFromTestDir - ], - }), - }; - - payloadData.payload.pull_request.changed_files = 1; - await robot.receive(payloadData); - }); - - it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); - }); - - it('should get modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - it('should not ping server job admin', () => { - expect(github.issues.createComment).not.toHaveBeenCalled(); - }); - }); - - describe('When job test file gets added', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - jobTestFile - ], - }), - }; - - payloadData.payload.pull_request.changed_files = 1; - await robot.receive(payloadData); - }); - - it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); - }); - - it('should get modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - it('should not ping server job admin', () => { - expect(github.issues.createComment).not.toHaveBeenCalled(); - }); - }); - - describe('When pull request has datastore label', () => { - beforeEach(async () => { - payloadData.payload.pull_request.labels = [{ - name: 'PR: Affects datastore layer' - }]; - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - nonJobFile, firstNewJobFileObj - ], - }), - }; - - payloadData.payload.pull_request.changed_files = 2; - await robot.receive(payloadData); - }); - - it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); - }); - - it('should not get modified files', () => { - expect(github.pulls.listFiles).not.toHaveBeenCalled(); - }); - - it('should not ping server job admin', () => { - expect(github.issues.createComment).not.toHaveBeenCalled(); - }); - }); - - describe('Returns appropriate job name', () => { - it('should return the correct job created in the file', () => { - let jobs = checkPullRequestJobModule.getNewJobsFromFile( - firstNewJobFileObj - ); - expect(jobs.length).toBe(1); - expect(jobs[0]).toBe('FirstTestJob'); - - jobs = checkPullRequestJobModule.getNewJobsFromFile(secondNewJobFileObj); - expect(jobs.length).toBe(1); - expect(jobs[0]).toBe('SecondTestJob'); - - jobs = checkPullRequestJobModule.getNewJobsFromFile( - modifiedExistingJobFileObj - ); - expect(jobs.length).toBe(1); - expect(jobs[0]).toBe('OppiabotContributionsJob'); - - jobs = checkPullRequestJobModule.getNewJobsFromFile(fileWithMultipleJobs); - expect(jobs.length).toBe(2); - expect(jobs[0]).toBe('TestJob'); - expect(jobs[1]).toBe('AnotherTestJob'); - - jobs = checkPullRequestJobModule.getNewJobsFromFile(jobTestFile); - expect(jobs.length).toBe(0); - - jobs = checkPullRequestJobModule.getNewJobsFromFile( - modifiedExistingJobFileObjWithJobClassInPatch); - expect(jobs.length).toBe(0); - - jobs = checkPullRequestJobModule.getNewJobsFromFile( - jobFileObjWithJobClassInPatchAndNewJob); - expect(jobs.length).toBe(1); - expect(jobs[0]).toBe('AnotherTestJob'); - }); - }); -}); From abb3daa7c028b838fc6d3e8c160de352eeb20b85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Mon, 3 Apr 2023 14:47:07 +0200 Subject: [PATCH 02/18] Add review files --- review/README | 6 ++ review/list.c | 163 ++++++++++++++++++++++++++++++++++ review/list.h | 158 +++++++++++++++++++++++++++++++++ review/list_utils.c | 53 +++++++++++ review/list_utils.h | 64 ++++++++++++++ review/main.c | 209 ++++++++++++++++++++++++++++++++++++++++++++ review/readline.c | 48 ++++++++++ review/readline.h | 18 ++++ 8 files changed, 719 insertions(+) create mode 100644 review/README create mode 100644 review/list.c create mode 100644 review/list.h create mode 100644 review/list_utils.c create mode 100644 review/list_utils.h create mode 100644 review/main.c create mode 100644 review/readline.c create mode 100644 review/readline.h diff --git a/review/README b/review/README new file mode 100644 index 00000000..e4c46f70 --- /dev/null +++ b/review/README @@ -0,0 +1,6 @@ +1. Download ZIP archive using https://is.muni.cz/go/1rlpne +2. Make sure that you're on main branch (git status) +3. Pull latest changes from remote (git pull) +4. Create new branch and change to (git checkout -b review) +5. Unzip the archive into review folder +6. Create new commit (git commit -m ) \ No newline at end of file diff --git a/review/list.c b/review/list.c new file mode 100644 index 00000000..960de9a8 --- /dev/null +++ b/review/list.c @@ -0,0 +1,163 @@ +#include "list.h" + +#include +#include + +void list_init(struct list *list, size_t element_size) +{ + assert(list != NULL); + assert(element_size > 0); + + list->head = list->tail = NULL; + list->elem_size = element_size; +} + +void list_destroy(struct list *list) +{ + assert(list != NULL); + + // «list_pop_front()» takes care of node removal + while (list_pop_front(list, NULL)) { + /* nop */; + } +} + +size_t list_size(const struct list *list) +{ + assert(list != NULL); + + size_t counter = 0U; + + // for loop ftw + for (const struct node *node = list->head; node != NULL; node = node->next) + ++counter; + + return counter; +} + +bool list_is_empty(const struct list *list) +{ + assert(list != NULL); + return list->head == NULL; +} + +static struct node *list_new_node(const struct list *list, const void *data) +{ + struct node *node = malloc(sizeof(struct node) + list->elem_size); + + if (node == NULL) + return NULL; + + memcpy(node->data, data, list->elem_size); + return node; +} + +static void list_attach(struct list *list, struct node *node, struct node *prev) +{ + node->prev = prev; + + if (prev != NULL) { + // if previous node exists, insert «node» between «prev» and «prev->next» + if (prev->next != NULL) + prev->next->prev = node; + + node->next = prev->next; + prev->next = node; + } else { + // otherwise add node before «list->head» + if (list->head != NULL) + list->head->prev = node; + + node->next = list->head; + } + + // update «list->head» and «list->tail» accordingly + if (node->prev == NULL) + list->head = node; + if (node->next == NULL) + list->tail = node; +} + +static struct node *list_detach(struct list *list, struct node *node) +{ + // remove bindings between «node» and its siblings + if (node->prev != NULL) + node->prev->next = node->next; + if (node->next != NULL) + node->next->prev = node->prev; + + // update «list->head» and «list->tail» accordingly + if (list->head == node) + list->head = node->next; + if (list->tail == node) + list->tail = node->prev; + + return node; +} + +bool list_push_back(struct list *list, const void *data) +{ + assert(list != NULL); + assert(data != NULL); + + // get a new node + struct node *node = list_new_node(list, data); + + if (node == NULL) + return false; + + // attach the node after «list->tail» + list_attach(list, node, list->tail); + return true; +} + +bool list_pop_back(struct list *list, void *data) +{ + assert(list != NULL); + + if (list_is_empty(list)) + return false; + + // detach «list->tail» node + struct node *node = list_detach(list, list->tail); + + if (data != NULL) + memcpy(data, node->data, list->elem_size); + + // the node is useless now + free(node); + return true; +} + +bool list_push_front(struct list *list, const void *data) +{ + assert(list != NULL); + assert(data != NULL); + + // this function works analogously to «list_push_back()» + struct node *node = list_new_node(list, data); + + if (node == NULL) + return false; + + // «prev» set to «NULL» means there is no previous node + list_attach(list, node, NULL); + return true; +} + +bool list_pop_front(struct list *list, void *data) +{ + assert(list != NULL); + + if (list_is_empty(list)) + return false; + + // this function works analogously to «list_pop_back()» + struct node *node = list_detach(list, list->head); + + if (data != NULL) + memcpy(data, node->data, list->elem_size); + + free(node); + return true; +} diff --git a/review/list.h b/review/list.h new file mode 100644 index 00000000..d6e45b5b --- /dev/null +++ b/review/list.h @@ -0,0 +1,158 @@ +#ifndef LIST_H +#define LIST_H + +/** + * \file list.h + * \author Roman Lacko + */ + +#include +#include + +//----------------------------------------------------------------------------- +// Features +//----------------------------------------------------------------------------- + +// Example implementation has the following macros enabled +#define LLIST_ENABLE_TWO_LINKS +#define LLIST_ENABLE_FAM +#define LLIST_ENABLE_ALL_ENDS + +//----------------------------------------------------------------------------- +// Types +//----------------------------------------------------------------------------- + +/** + * \brief Doubly linked list node. + */ +struct node +{ + struct node *next; /**< next node */ + struct node *prev; /**< previous node */ + + char data[]; /**< flexible array member */ +}; + +/** + * \brief Doubly linked list. + */ +struct list +{ + struct node *head; /**< list's first node */ + struct node *tail; /**< list's last node */ + size_t elem_size; /**< size of stored elements */ +}; + +//----------------------------------------------------------------------------- +// Consistency requirements +//----------------------------------------------------------------------------- + +/* + * Let «P» be a property that a list has if and only if it satisfies + * the following requirements: + * + * A) ‹list->head› is ‹NULL› if and only if the ‹list->tail› is ‹NULL›, + * B) if ‹list->head› is not ‹NULL›, then ‹head->prev› is ‹NULL›, + * C) if ‹list->tail› is not ‹NULL›, then ‹tail->next› is ‹NULL›, + * + * Each of the following functions must satisfy this condition: + * + * If the function's argument is a list with property «P», then + * the list also has the property «P» after the function ends. + */ + +//----------------------------------------------------------------------------- +// Note +//----------------------------------------------------------------------------- + +/* + * Unless stated otherwise, ‹NULL› value passed as a parameter to any of + * the following functions causes undefined behaviour. This is not tested + * by the included tests. + * + * Use ‹assert()› to check for these conditions. + */ + +//----------------------------------------------------------------------------- +// Functions +//----------------------------------------------------------------------------- + +/** + * \brief Initializes the list structure. + * + * The result of this operation is an empty list. + * + * \note The behaviour of this function is undefined if \p element_size is zero. + * + * \param element_size size (in bytes) of stored elements + */ +void list_init(struct list *list, size_t element_size); + +/** + * \brief Releases all resources associated with the \p list. + */ +void list_destroy(struct list *list); + +/** + * \brief Returns the number of elements stored in the list. + */ +size_t list_size(const struct list *list); + +/** + * \brief Predicate that tells wheter the \a list is empty or not. + * + * \returns \c true if the \a list is empty, \c false otherwise + */ +bool list_is_empty(const struct list *list); + +/** + * \brief Inserts a new element after the last element in the list. + * + * The element will contain a copy of the memory pointed to by the + * \p data parameter. + * + * \param data pointer to data to be stored + * \returns \c true if the operation was successful, \c false otherwise + */ +bool list_push_back(struct list *list, const void *data); + +/** + * \brief Removes and returns the last element stored in the list. + * + * If \p data is not \c NULL, contents of the node data will be copied + * to the memory pointed to by \p data pointer. + * + * \note If \p data is set to \c NULL, the element is simply discarded. + * \note If the list is empty, the function must not change the memory + * pointed to by \p data. + * + * \returns \c true if the operation was successful, \c false otherwise + */ +bool list_pop_back(struct list *list, void *data); + +/** + * \brief Inserts a new element before the first element in the list. + * + * The element will contain a copy of the memory pointed to by the + * \p data parameter. + * + * \param data pointer to data to be stored + * \return \c true if the operation was successful, \c false otherwise + */ +bool list_push_front(struct list *list, const void *data); + +/** + * \brief Removes and returns the first element stored in the list. + * + * If \p data is not \c NULL, contents of the node data will be copied + * to the memory pointed to by \p data pointer. + * + * \note If \p list is empty, the operation fails. + * \note If the list is empty, the function must not change the memory + * pointed to by \p data. + * + * \returns \c true if the operation was successful, \c false otherwise + */ +bool list_pop_front(struct list *list, void *data); + +#endif // LIST_H diff --git a/review/list_utils.c b/review/list_utils.c new file mode 100644 index 00000000..7dcc0e5c --- /dev/null +++ b/review/list_utils.c @@ -0,0 +1,53 @@ +#include "list_utils.h" + +#include +#include + +#include "list.h" + +// swaps values between two nodes +static void swap_node_data(const struct list *list, struct node *a, struct node *b) +{ + assert(list != NULL); + assert(a != NULL); + assert(b != NULL); + + char tmp; + for (size_t byte = 0; byte < list->elem_size; ++byte) { + tmp = a->data[byte]; + a->data[byte] = b->data[byte]; + b->data[byte] = tmp; + } +} + +void list_for_each(struct list *list, action f) +{ + assert(list != NULL); + assert(f != NULL); + + for (struct node *node = list->head; node != NULL; node = node->next) + f(node->data); +} + +void list_sort(struct list *list, comparator cmp) +{ + assert(list != NULL); + assert(cmp != NULL); + + // this implementation uses improved bubblesort that counts swaps + // in each iteration + for (struct node *right = list->tail; right != NULL; right = right->prev) { + size_t swaps = 0U; + + for (struct node *swp = list->head; swp != right; swp = swp->next) { + if (cmp(swp->data, swp->next->data) > 0) { + swap_node_data(list, swp, swp->next); + ++swaps; + } + } + + // if there were no swaps, the list is sorted and we may end + if (swaps == 0U) + return; + } +} diff --git a/review/list_utils.h b/review/list_utils.h new file mode 100644 index 00000000..2faf6476 --- /dev/null +++ b/review/list_utils.h @@ -0,0 +1,64 @@ +#ifndef LIST_UTILS_H +#define LIST_UTILS_H + +/** + * \file list_utils.h + * \author ... + */ + +#include + +#include "list.h" + +//----------------------------------------------------------------------------- +// Types +//----------------------------------------------------------------------------- + +/** + * \brief Predicate. + */ +typedef bool (*predicate)(const void *); + +/** + * \brief Comparator. + * + * Let a1 < a2 denote that argument 1 should be before argument 2 in the + * ordering implemented by this function; then the comparator returns number + *
    + *
  • < 0 if a1 < a2
    • + *
  • > 0 if a1 > a2
    • + *
  • 0 otherwise
    • + *
+ */ +typedef int (*comparator)(const void *, const void *); + +/** + * \brief Action. + */ +typedef void (*action)(void *); + +//----------------------------------------------------------------------------- +// Enumeration +//----------------------------------------------------------------------------- + +/** + * \brief Calls function \p f on each element from head to tail. + * + * \param list the list + * \param f the function to call + */ +void list_for_each(struct list *list, action f); + +//----------------------------------------------------------------------------- +// Sorting +//----------------------------------------------------------------------------- + +/** + * \brief Sorts all elements in the list using \p cmp comparator. + * + * \param list the list + * \param cmp the comparator + */ +void list_sort(struct list *list, comparator cmp); + +#endif // LIST_UTILS_H diff --git a/review/main.c b/review/main.c new file mode 100644 index 00000000..87e7c481 --- /dev/null +++ b/review/main.c @@ -0,0 +1,209 @@ +#include +#include +#include + +#include "readline.h" +#include "list.h" +#include "list_utils.h" + +const char *versions[5] = { + "Version 1.2.5", + "Version 2.2.3", + NULL, + "Version 4.2.4", + "Version 5.2.2", +}; +const char* beta_version = "Version 6.0.0beta"; +const char* mobile_version = "Version 5.2.2mobile"; + +struct line_info +{ + int number; + char *line; + char *text; +}; + +void free_line_info(void *ptr) +{ + struct line_info *info = (struct line_info *) ptr; + free(info->line); +} + +int line_info_cmp(const void *a, const void *b) +{ + return ((struct line_info *) a)->number - ((struct line_info *) b)->number; +} + +int read_lines(struct list *list, FILE *in) +{ + char *line; + while ((line = readline(in)) != NULL) { + struct line_info info; + + info.line = line; + + char *num = strtok(line, ":"); + char *endptr; + + info.number = strtol(num, &endptr, 10); + + if (*endptr != '\0') { + fprintf(stderr, "on line %zu: invalid number '%s'\n", list_size(list) + 1, num); + return 0x00; + } + + info.text = strtok(NULL, ""); + + if (!list_push_back(list, &info)) { + perror("list_push_back"); + free_line_info(&info); + return 0x00; + } + } + + return 0x20; +} + +void write_line(void *ptr) +{ + printf("%s", ((struct line_info *) ptr)->text); +} + +int mode_a() { + printf("Mode A\n"); + + char *input_file = "input.in"; + FILE *in; + + in = fopen(input_file, "r"); + + int status = EXIT_FAILURE; + struct list list; + list_init(&list, sizeof(struct line_info)); + + if ((status = read_lines(&list, in)) == 0x20) { + list_sort(&list, &line_info_cmp); + list_for_each(&list, &write_line); + status = EXIT_SUCCESS; + } + + // ‹list_destroy()› will not clear memory correctly! + struct line_info item; + while (list_pop_front(&list, &item)) free_line_info(&item); + + list_destroy(&list); + + return status; +} + +int mode_b() { + printf("Pick a version: \n1-5 - Version 1.x.y - 5.x.y\nb - Version 5.x.yb\nm - Version 1.x.ym"); + int version = getchar(); + if ((version >= '1' && version <= '5') || version == 'm' || version == 'b') { + if (version == '1') { + printf("%s", versions[0]); + if (versions[0] == NULL) { + printf("Error"); + return EXIT_FAILURE; + } + // TODO: load the into config + } else if (version == '2') { + printf("%s", versions[1]); + if (versions[1] == NULL) { + printf("Error"); + return EXIT_FAILURE; + } + // TODO: load the into config + } else if (version == '3') { + printf("%s", versions[2]); + if (versions[2] == NULL) { + printf("Error"); + return EXIT_FAILURE; + } + // TODO: load the into config + } else if (version == '4') { + printf("%s", versions[3]); + if (versions[3] == NULL) { + printf("Error"); + return EXIT_FAILURE; + } + // TODO: load the into config + } else if (version == '5') { + printf("%s", versions[4]); + if (versions[4] == NULL) { + printf("Error"); + return EXIT_FAILURE; + } + // TODO: load the into config + } else if (version == 'b') { + printf("%s", beta_version); + // TODO: load the into config + } else if (version == 'm') { + printf("%s", mobile_version); + // TODO: load the into config + } + } else { + printf("Error"); + return EXIT_FAILURE; + } +} + +void sort_data(int data[100]) { + for (size_t i = 0; i < 100; i++) { + for (size_t j = i; i < 100; i++) { + if (data[i] > data[j]) { + int tmp = data[i]; + data[i] = data[j]; + data[j] = tmp; + } + } + } +} + +int mode_c() { + int *data = calloc(100, sizeof(int)); + if (data == NULL) { + printf("Error"); + return EXIT_FAILURE; + } + for (size_t i = 1; i <= 100; i++) { + if (i % 25 == 0) { + if (i % 5 == 0) { + data[i - 1] = 0x123; // TODO assign correct data + if (data[i - 2] == 0) { + data[i - 2] += 0x456; // TODO assign correct data + continue; + } + } else + continue; + } + if (i % 20 == 0) { + data[i - 1] = 0x456; // TODO assign correct data + if (i % 10 == 0) { + data[i - 2] += 0x789; // TODO assign correct data + continue; + } + } + if (i % 5 == 0 && !(i % 10 == 0)) { + continue; + } + } + sort_data(data); +} + +int main(int argc, char *argv[]) { + printf("Pick a mode: \na - Mode A\nb - Mode B\nc - Mode C\n"); + int mode = getchar(); + if (mode < 97 || mode > 100) { + printf("Error"); + return EXIT_FAILURE; + } + if (mode == 'a') { + mode_a(); + } else if (mode == 'b') { + mode_b(); + } else if (mode == 'c') { + mode_c(); + } + return EXIT_SUCCESS; +} diff --git a/review/readline.c b/review/readline.c new file mode 100644 index 00000000..e35c76f2 --- /dev/null +++ b/review/readline.c @@ -0,0 +1,48 @@ +#include "readline.h" + +#include +#include +#include +#include +#include + +char *readline(FILE *file) +{ + assert(file != NULL); + size_t capacity = 0u; + size_t length = 0u; + size_t chunk_size = 64u; + size_t offset = 0u; + char *memory = NULL; + char *last_chunk; + + do { + char *tmp; + + // increase buffer size + capacity += chunk_size; + if ((tmp = realloc(memory, capacity)) == NULL) { + free(memory); + return NULL; + } + + // read next chunk + memory = tmp; + last_chunk = memory + length - offset; + + if (fgets(last_chunk, chunk_size + offset, file) == NULL) + break; + + offset = 1; + length += chunk_size; + + // found newline? + } while (strchr(last_chunk, '\n') == NULL); + + if (length == 0u || ferror(file)) { + free(memory); + return NULL; + } + + return memory; +} diff --git a/review/readline.h b/review/readline.h new file mode 100644 index 00000000..c4a46bff --- /dev/null +++ b/review/readline.h @@ -0,0 +1,18 @@ +#ifndef READLINE_H +#define READLINE_H + +#include + +/** + * Reads an entire line from the input. + * + * \note The caller is required to call \c free() on the returned pointer + * at some point before exiting the program. + * + * \param file input file + * \return pointer to the newly allocated memory containing the lin + * or \c NULL on failure + */ +char *readline(FILE *file); + +#endif // READLINE_H From c5a290c0b91aee008809f42c928e2ef35b360bcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 13:59:56 +0200 Subject: [PATCH 03/18] Changes --- .github/workflows/frontend_tests.yml | 2 +- index.js | 3 +- lib/checkPullRequestJob.js | 116 +++++++-------------------- lib/utils.js | 2 +- spec/apiForSheetsSpec.js | 16 +++- spec/checkCriticalPullRequestSpec.js | 4 +- spec/checkForNewCodeownerSpec.js | 4 +- spec/checkMergeConflictSpec.js | 4 +- spec/checkPullRequestBranchSpec.js | 5 +- spec/checkPullRequestLabelsSpec.js | 4 +- spec/checkPullRequestTemplateSpec.js | 4 +- 11 files changed, 61 insertions(+), 103 deletions(-) diff --git a/.github/workflows/frontend_tests.yml b/.github/workflows/frontend_tests.yml index c9e89b30..d2cd4f58 100644 --- a/.github/workflows/frontend_tests.yml +++ b/.github/workflows/frontend_tests.yml @@ -17,7 +17,7 @@ jobs: - uses: actions/checkout@v2 - uses: actions/setup-node@v3 with: - node-version: '14' + node-version: '16' - name: Install dependencies run: npm install - name: Run frontend tests diff --git a/index.js b/index.js index c4eeddb4..d689a3ff 100644 --- a/index.js +++ b/index.js @@ -84,7 +84,8 @@ const runChecks = async (context, checkEvent) => { ); break; case constants.jobCheck: - callable.push(checkPullRequestJobModule.checkForNewJob(context)); + callable.push( + checkPullRequestJobModule.checkForModificationsToFiles(context)); break; case constants.cronJobCheck: callable.push(checkCronJobModule.checkForNewCronJob(context)); diff --git a/lib/checkPullRequestJob.js b/lib/checkPullRequestJob.js index 2bf0999d..7b1fc2bb 100644 --- a/lib/checkPullRequestJob.js +++ b/lib/checkPullRequestJob.js @@ -16,113 +16,56 @@ const { serverJobAdmins } = require('../userWhitelist.json'); const { - JOBS_AND_FETURES_TESTING_WIKI_LINK, + JOBS_AND_FEATURES_TESTING_WIKI_LINK, DATASTORE_LABEL, getAllChangedFiles, - getNameString, - getNewItemsFromFileByRegex, hasDatastoreLabel, pingAndAssignUsers } = require('./utils'); -const REGISTRY_FILENAME = 'jobs_registry.py'; const JOBS_DIR_PREFIX = 'core/jobs/'; -const JOB_REGEX = new RegExp( - [ - '(?\\+)(?class\\s)', - '(?[a-zA-Z]{2,80})(?Job)(?\\()' - ].join('') -); +const PLATFORM_DIR_PREFIX = 'core/platform/'; /** * @param {string} filename */ -const isInJobsDir = (filename) => { - return filename.startsWith(JOBS_DIR_PREFIX); -}; - -/** - * @param {import('probot').Octokit.PullsListFilesResponseItem} file - */ -const getNewJobsFromFile = (file) => { - return getNewItemsFromFileByRegex(JOB_REGEX, file); -}; - -/** - * @param {import('probot').Octokit.PullsListFilesResponseItem} file - */ -const addsNewJob = (file) => { - const newJobs = getNewJobsFromFile(file); - return newJobs.length > 0; -}; - -/** - * @param {import('probot').Octokit.PullsListFilesResponseItem[]} newJobFiles - */ -const getJobNameString = (newJobFiles) => { - return getNameString( - newJobFiles, - { - singular: 'job', - plural: 'jobs' - }, - JOB_REGEX +const isInDirsThatShouldBeTested = (filename) => { + return ( + filename.startsWith(JOBS_DIR_PREFIX) || + filename.startsWith(PLATFORM_DIR_PREFIX) ); }; /** - * @param {import('probot').Octokit.PullsListFilesResponseItem[]} newJobFiles - * @param {import('probot').Octokit.PullsListFilesResponseItem[]} changedFiles * @param {string} prAuthor - Author of the Pull Request */ -const getCommentBody = (newJobFiles, changedFiles, prAuthor) => { - const jobNameString = getJobNameString(newJobFiles); +const getCommentBody = (prAuthor) => { const newLineFeed = '
'; const serverJobsForm = 'server jobs form'.link( 'https://goo.gl/forms/XIj00RJ2h5L55XzU2'); const wikiLinkText = 'this guide'.link( JOBS_AND_FETURES_TESTING_WIKI_LINK); - // This function will never be called when there are no job files. - if (newJobFiles.length === 1) { - const serverAdminPing = ( - 'Hi @' + serverJobAdmins.join(', @') + - ', PTAL at this PR, ' + 'it adds a new job.' + jobNameString - ); - - const prAuthorPing = ( - 'Also @' + prAuthor + ', ' + - 'please make sure to fill in the ' + serverJobsForm + ' ' + - 'for the new job to be tested on the backup server. ' + - 'This PR can be merged only after the test run is successful. ' + - 'Please refer to ' + wikiLinkText + ' for details.' - ); - return ( - serverAdminPing + newLineFeed + prAuthorPing + newLineFeed + 'Thanks!' - ); - } else { - const serverAdminPing = ( - 'Hi @' + serverJobAdmins.join(', @') + ', PTAL at this PR, ' + - 'it adds new jobs.' + jobNameString - ); - - const prAuthorPing = ( - 'Also @' + prAuthor + ', ' + - 'please make sure to fill in the ' + serverJobsForm + ' ' + - 'for the new jobs to be tested on the backup server. ' + - 'This PR can be merged only after the test run is successful. ' + - 'Please refer to ' + wikiLinkText + ' for details.' - ); - return ( - serverAdminPing + newLineFeed + prAuthorPing + newLineFeed + 'Thanks!' - ); - } + const serverAdminPing = ( + 'Hi @' + serverJobAdmins.join(', @') + + ', PTAL at this PR, it modifies files in jobs or platform folders.' + ); + const prAuthorPing = ( + 'Also @' + prAuthor + ', ' + + 'please make sure to fill in the ' + serverJobsForm + ' ' + + 'for the new job or feature to be tested on the backup server. ' + + 'This PR can be merged only after the test run is successful. ' + + 'Please refer to ' + wikiLinkText + ' for details.' + ); + return ( + serverAdminPing + newLineFeed + prAuthorPing + newLineFeed + 'Thanks!' + ); }; /** * @param {import('probot').Context} context */ -const checkForNewJob = async (context) => { +const checkForModificationsToFiles = async (context) => { /** * @type {import('probot').Octokit.PullsGetResponse} pullRequest */ @@ -131,16 +74,12 @@ const checkForNewJob = async (context) => { const changedFiles = await getAllChangedFiles(context); // Get new jobs that were created in the PR. - const newJobFiles = changedFiles.filter((file) => { - return isInJobsDir(file.filename) && addsNewJob(file); + const modifiedFilesToBeTested = changedFiles.filter((file) => { + return isInDirsThatShouldBeTested(file.filename); }); - if (newJobFiles.length > 0) { - const commentBody = getCommentBody( - newJobFiles, - changedFiles, - pullRequest.user.login - ); + if (modifiedFilesToBeTested.length > 0) { + const commentBody = getCommentBody(pullRequest.user.login); await pingAndAssignUsers( context, @@ -159,8 +98,7 @@ const checkForNewJob = async (context) => { }; module.exports = { - checkForNewJob, - getNewJobsFromFile, + checkForModificationsToFiles, hasDatastoreLabel, DATASTORE_LABEL }; diff --git a/lib/utils.js b/lib/utils.js index 302dfa32..7da3f697 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -448,7 +448,7 @@ module.exports = { DATASTORE_LABEL, HOTFIX_LABEL, OLD_BUILD_LABEL, - JOBS_AND_FETURES_TESTING_WIKI_LINK, + JOBS_AND_FEATURES_TESTING_WIKI_LINK, getAllChangedFiles, getNameString, getNewItemsFromFileByRegex, diff --git a/spec/apiForSheetsSpec.js b/spec/apiForSheetsSpec.js index 649bdb8b..223e5f64 100644 --- a/spec/apiForSheetsSpec.js +++ b/spec/apiForSheetsSpec.js @@ -52,7 +52,9 @@ describe('Api For Sheets Module', () => { beforeEach(function (done) { spyOn(scheduler, 'createScheduler').and.callFake(() => { }); - spyOn(checkPullRequestJobModule, 'checkForNewJob').and.callFake(() => { }); + spyOn( + checkPullRequestJobModule, 'checkForModificationsToFiles' + ).and.callFake(() => { }); spyOn(checkPullRequestBranchModule, 'checkBranch').and.callFake(() => { }); spyOn(checkCriticalPullRequestModule, 'checkIfPRAffectsDatastoreLayer') .and.callFake(() => { }); @@ -124,7 +126,9 @@ describe('Api For Sheets Module', () => { it('should call other checks', () => { expect(checkPullRequestBranchModule.checkBranch).toHaveBeenCalled(); - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + expect( + checkPullRequestJobModule.checkForModificationsToFiles + ).toHaveBeenCalled(); }); it('should be called for the given payload', () => { @@ -432,7 +436,9 @@ describe('Api For Sheets Module', () => { it('should not call any checks', () => { expect(checkPullRequestBranchModule.checkBranch).not.toHaveBeenCalled(); expect(apiForSheetsModule.checkClaStatus).not.toHaveBeenCalled(); - expect(checkPullRequestJobModule.checkForNewJob).not.toHaveBeenCalled(); + expect( + checkPullRequestJobModule.checkForModificationsToFiles + ).not.toHaveBeenCalled(); }); }); @@ -450,7 +456,9 @@ describe('Api For Sheets Module', () => { it('should not call any checks', () => { expect(checkPullRequestBranchModule.checkBranch).not.toHaveBeenCalled(); expect(apiForSheetsModule.checkClaStatus).not.toHaveBeenCalled(); - expect(checkPullRequestJobModule.checkForNewJob).not.toHaveBeenCalled(); + expect( + checkPullRequestJobModule.checkForModificationsToFiles + ).not.toHaveBeenCalled(); }); }); }); diff --git a/spec/checkCriticalPullRequestSpec.js b/spec/checkCriticalPullRequestSpec.js index 110864ba..859d7c10 100644 --- a/spec/checkCriticalPullRequestSpec.js +++ b/spec/checkCriticalPullRequestSpec.js @@ -191,7 +191,9 @@ describe('Critical Pull Request Spec', () => { app = robot.load(oppiaBot); spyOn(app, 'auth').and.resolveTo(github); - spyOn(checkPullRequestJobModule, 'checkForNewJob').and.callFake(() => { }); + spyOn( + checkPullRequestJobModule, 'checkForModificationsToFiles' + ).and.callFake(() => { }); spyOn(checkCronJobModule, 'checkForNewCronJob').and.callFake(() => { }); spyOn(apiForSheetsModule, 'checkClaStatus').and.callFake(() => { }); spyOn(checkPullRequestBranchModule, 'checkBranch').and.callFake(() => { }); diff --git a/spec/checkForNewCodeownerSpec.js b/spec/checkForNewCodeownerSpec.js index 1afedab8..5e87c609 100644 --- a/spec/checkForNewCodeownerSpec.js +++ b/spec/checkForNewCodeownerSpec.js @@ -199,7 +199,9 @@ describe('Check for new code owner', () => { app = robot.load(oppiaBot); spyOn(app, 'auth').and.resolveTo(github); - spyOn(checkPullRequestJobModule, 'checkForNewJob').and.callFake(() => { }); + spyOn( + checkPullRequestJobModule, 'checkForModificationsToFiles' + ).and.callFake(() => { }); spyOn(checkCronJobModule, 'checkForNewCronJob').and.callFake(() => { }); spyOn( checkCriticalPullRequestModule, diff --git a/spec/checkMergeConflictSpec.js b/spec/checkMergeConflictSpec.js index 16ce71a7..59c0ad53 100644 --- a/spec/checkMergeConflictSpec.js +++ b/spec/checkMergeConflictSpec.js @@ -74,7 +74,9 @@ describe('Merge Conflict Check', () => { app = robot.load(oppiaBot); spyOn(app, 'auth').and.resolveTo(github); - spyOn(checkPullRequestJobModule, 'checkForNewJob').and.callFake(() => { }); + spyOn( + checkPullRequestJobModule, 'checkForModificationsToFiles' + ).and.callFake(() => { }); spyOn(checkCronJobModule, 'checkForNewCronJob').and.callFake(() => { }); spyOn(newCodeOwnerModule, 'checkForNewCodeowner').and.callFake(() => { }); spyOn( diff --git a/spec/checkPullRequestBranchSpec.js b/spec/checkPullRequestBranchSpec.js index befcdb02..81efeb9a 100644 --- a/spec/checkPullRequestBranchSpec.js +++ b/spec/checkPullRequestBranchSpec.js @@ -49,8 +49,9 @@ describe('Pull Request Branch Check', () => { // Spy on other modules that will be triggered by the payload. spyOn(apiForSheetsModule, 'checkClaStatus').and.callFake(() => { }); - spyOn(checkPullRequestJobModule, 'checkForNewJob') - .and.callFake(() => { }); + spyOn( + checkPullRequestJobModule, 'checkForModificationsToFiles' + ).and.callFake(() => { }); spyOn(checkCriticalPullRequestModule, 'checkIfPRAffectsDatastoreLayer') .and.callFake(() => { }); spyOn(checkPullRequestTemplateModule, 'checkTemplate') diff --git a/spec/checkPullRequestLabelsSpec.js b/spec/checkPullRequestLabelsSpec.js index 50662e90..15df0f1f 100644 --- a/spec/checkPullRequestLabelsSpec.js +++ b/spec/checkPullRequestLabelsSpec.js @@ -81,7 +81,9 @@ describe('Pull Request Label Check', () => { app = robot.load(oppiaBot); spyOn(app, 'auth').and.resolveTo(github); - spyOn(checkPullRequestJobModule, 'checkForNewJob').and.callFake(() => { }); + spyOn( + checkPullRequestJobModule, 'checkForModificationsToFiles' + ).and.callFake(() => { }); spyOn( checkCriticalPullRequestModule, 'checkIfPRAffectsDatastoreLayer' ).and.callFake(() => { }); diff --git a/spec/checkPullRequestTemplateSpec.js b/spec/checkPullRequestTemplateSpec.js index 02d393a0..6b60548a 100644 --- a/spec/checkPullRequestTemplateSpec.js +++ b/spec/checkPullRequestTemplateSpec.js @@ -289,7 +289,9 @@ describe('Pull Request Template', () => { app = robot.load(oppiaBot); spyOn(app, 'auth').and.resolveTo(github); - spyOn(checkPullRequestJobModule, 'checkForNewJob').and.callFake(() => { }); + spyOn( + checkPullRequestJobModule, 'checkForModificationsToFiles' + ).and.callFake(() => { }); spyOn(checkCronJobModule, 'checkForNewCronJob').and.callFake(() => { }); spyOn(apiForSheetsModule, 'checkClaStatus').and.callFake(() => { }); spyOn( From 4310dd1eefa633e49412f0ee6ee66386600dec1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 14:02:16 +0200 Subject: [PATCH 04/18] Changes --- review/README | 6 -- review/list.c | 163 ---------------------------------- review/list.h | 158 --------------------------------- review/list_utils.c | 53 ----------- review/list_utils.h | 64 -------------- review/main.c | 209 -------------------------------------------- review/readline.c | 48 ---------- review/readline.h | 18 ---- 8 files changed, 719 deletions(-) delete mode 100644 review/README delete mode 100644 review/list.c delete mode 100644 review/list.h delete mode 100644 review/list_utils.c delete mode 100644 review/list_utils.h delete mode 100644 review/main.c delete mode 100644 review/readline.c delete mode 100644 review/readline.h diff --git a/review/README b/review/README deleted file mode 100644 index e4c46f70..00000000 --- a/review/README +++ /dev/null @@ -1,6 +0,0 @@ -1. Download ZIP archive using https://is.muni.cz/go/1rlpne -2. Make sure that you're on main branch (git status) -3. Pull latest changes from remote (git pull) -4. Create new branch and change to (git checkout -b review) -5. Unzip the archive into review folder -6. Create new commit (git commit -m ) \ No newline at end of file diff --git a/review/list.c b/review/list.c deleted file mode 100644 index 960de9a8..00000000 --- a/review/list.c +++ /dev/null @@ -1,163 +0,0 @@ -#include "list.h" - -#include -#include - -void list_init(struct list *list, size_t element_size) -{ - assert(list != NULL); - assert(element_size > 0); - - list->head = list->tail = NULL; - list->elem_size = element_size; -} - -void list_destroy(struct list *list) -{ - assert(list != NULL); - - // «list_pop_front()» takes care of node removal - while (list_pop_front(list, NULL)) { - /* nop */; - } -} - -size_t list_size(const struct list *list) -{ - assert(list != NULL); - - size_t counter = 0U; - - // for loop ftw - for (const struct node *node = list->head; node != NULL; node = node->next) - ++counter; - - return counter; -} - -bool list_is_empty(const struct list *list) -{ - assert(list != NULL); - return list->head == NULL; -} - -static struct node *list_new_node(const struct list *list, const void *data) -{ - struct node *node = malloc(sizeof(struct node) + list->elem_size); - - if (node == NULL) - return NULL; - - memcpy(node->data, data, list->elem_size); - return node; -} - -static void list_attach(struct list *list, struct node *node, struct node *prev) -{ - node->prev = prev; - - if (prev != NULL) { - // if previous node exists, insert «node» between «prev» and «prev->next» - if (prev->next != NULL) - prev->next->prev = node; - - node->next = prev->next; - prev->next = node; - } else { - // otherwise add node before «list->head» - if (list->head != NULL) - list->head->prev = node; - - node->next = list->head; - } - - // update «list->head» and «list->tail» accordingly - if (node->prev == NULL) - list->head = node; - if (node->next == NULL) - list->tail = node; -} - -static struct node *list_detach(struct list *list, struct node *node) -{ - // remove bindings between «node» and its siblings - if (node->prev != NULL) - node->prev->next = node->next; - if (node->next != NULL) - node->next->prev = node->prev; - - // update «list->head» and «list->tail» accordingly - if (list->head == node) - list->head = node->next; - if (list->tail == node) - list->tail = node->prev; - - return node; -} - -bool list_push_back(struct list *list, const void *data) -{ - assert(list != NULL); - assert(data != NULL); - - // get a new node - struct node *node = list_new_node(list, data); - - if (node == NULL) - return false; - - // attach the node after «list->tail» - list_attach(list, node, list->tail); - return true; -} - -bool list_pop_back(struct list *list, void *data) -{ - assert(list != NULL); - - if (list_is_empty(list)) - return false; - - // detach «list->tail» node - struct node *node = list_detach(list, list->tail); - - if (data != NULL) - memcpy(data, node->data, list->elem_size); - - // the node is useless now - free(node); - return true; -} - -bool list_push_front(struct list *list, const void *data) -{ - assert(list != NULL); - assert(data != NULL); - - // this function works analogously to «list_push_back()» - struct node *node = list_new_node(list, data); - - if (node == NULL) - return false; - - // «prev» set to «NULL» means there is no previous node - list_attach(list, node, NULL); - return true; -} - -bool list_pop_front(struct list *list, void *data) -{ - assert(list != NULL); - - if (list_is_empty(list)) - return false; - - // this function works analogously to «list_pop_back()» - struct node *node = list_detach(list, list->head); - - if (data != NULL) - memcpy(data, node->data, list->elem_size); - - free(node); - return true; -} diff --git a/review/list.h b/review/list.h deleted file mode 100644 index d6e45b5b..00000000 --- a/review/list.h +++ /dev/null @@ -1,158 +0,0 @@ -#ifndef LIST_H -#define LIST_H - -/** - * \file list.h - * \author Roman Lacko - */ - -#include -#include - -//----------------------------------------------------------------------------- -// Features -//----------------------------------------------------------------------------- - -// Example implementation has the following macros enabled -#define LLIST_ENABLE_TWO_LINKS -#define LLIST_ENABLE_FAM -#define LLIST_ENABLE_ALL_ENDS - -//----------------------------------------------------------------------------- -// Types -//----------------------------------------------------------------------------- - -/** - * \brief Doubly linked list node. - */ -struct node -{ - struct node *next; /**< next node */ - struct node *prev; /**< previous node */ - - char data[]; /**< flexible array member */ -}; - -/** - * \brief Doubly linked list. - */ -struct list -{ - struct node *head; /**< list's first node */ - struct node *tail; /**< list's last node */ - size_t elem_size; /**< size of stored elements */ -}; - -//----------------------------------------------------------------------------- -// Consistency requirements -//----------------------------------------------------------------------------- - -/* - * Let «P» be a property that a list has if and only if it satisfies - * the following requirements: - * - * A) ‹list->head› is ‹NULL› if and only if the ‹list->tail› is ‹NULL›, - * B) if ‹list->head› is not ‹NULL›, then ‹head->prev› is ‹NULL›, - * C) if ‹list->tail› is not ‹NULL›, then ‹tail->next› is ‹NULL›, - * - * Each of the following functions must satisfy this condition: - * - * If the function's argument is a list with property «P», then - * the list also has the property «P» after the function ends. - */ - -//----------------------------------------------------------------------------- -// Note -//----------------------------------------------------------------------------- - -/* - * Unless stated otherwise, ‹NULL› value passed as a parameter to any of - * the following functions causes undefined behaviour. This is not tested - * by the included tests. - * - * Use ‹assert()› to check for these conditions. - */ - -//----------------------------------------------------------------------------- -// Functions -//----------------------------------------------------------------------------- - -/** - * \brief Initializes the list structure. - * - * The result of this operation is an empty list. - * - * \note The behaviour of this function is undefined if \p element_size is zero. - * - * \param element_size size (in bytes) of stored elements - */ -void list_init(struct list *list, size_t element_size); - -/** - * \brief Releases all resources associated with the \p list. - */ -void list_destroy(struct list *list); - -/** - * \brief Returns the number of elements stored in the list. - */ -size_t list_size(const struct list *list); - -/** - * \brief Predicate that tells wheter the \a list is empty or not. - * - * \returns \c true if the \a list is empty, \c false otherwise - */ -bool list_is_empty(const struct list *list); - -/** - * \brief Inserts a new element after the last element in the list. - * - * The element will contain a copy of the memory pointed to by the - * \p data parameter. - * - * \param data pointer to data to be stored - * \returns \c true if the operation was successful, \c false otherwise - */ -bool list_push_back(struct list *list, const void *data); - -/** - * \brief Removes and returns the last element stored in the list. - * - * If \p data is not \c NULL, contents of the node data will be copied - * to the memory pointed to by \p data pointer. - * - * \note If \p data is set to \c NULL, the element is simply discarded. - * \note If the list is empty, the function must not change the memory - * pointed to by \p data. - * - * \returns \c true if the operation was successful, \c false otherwise - */ -bool list_pop_back(struct list *list, void *data); - -/** - * \brief Inserts a new element before the first element in the list. - * - * The element will contain a copy of the memory pointed to by the - * \p data parameter. - * - * \param data pointer to data to be stored - * \return \c true if the operation was successful, \c false otherwise - */ -bool list_push_front(struct list *list, const void *data); - -/** - * \brief Removes and returns the first element stored in the list. - * - * If \p data is not \c NULL, contents of the node data will be copied - * to the memory pointed to by \p data pointer. - * - * \note If \p list is empty, the operation fails. - * \note If the list is empty, the function must not change the memory - * pointed to by \p data. - * - * \returns \c true if the operation was successful, \c false otherwise - */ -bool list_pop_front(struct list *list, void *data); - -#endif // LIST_H diff --git a/review/list_utils.c b/review/list_utils.c deleted file mode 100644 index 7dcc0e5c..00000000 --- a/review/list_utils.c +++ /dev/null @@ -1,53 +0,0 @@ -#include "list_utils.h" - -#include -#include - -#include "list.h" - -// swaps values between two nodes -static void swap_node_data(const struct list *list, struct node *a, struct node *b) -{ - assert(list != NULL); - assert(a != NULL); - assert(b != NULL); - - char tmp; - for (size_t byte = 0; byte < list->elem_size; ++byte) { - tmp = a->data[byte]; - a->data[byte] = b->data[byte]; - b->data[byte] = tmp; - } -} - -void list_for_each(struct list *list, action f) -{ - assert(list != NULL); - assert(f != NULL); - - for (struct node *node = list->head; node != NULL; node = node->next) - f(node->data); -} - -void list_sort(struct list *list, comparator cmp) -{ - assert(list != NULL); - assert(cmp != NULL); - - // this implementation uses improved bubblesort that counts swaps - // in each iteration - for (struct node *right = list->tail; right != NULL; right = right->prev) { - size_t swaps = 0U; - - for (struct node *swp = list->head; swp != right; swp = swp->next) { - if (cmp(swp->data, swp->next->data) > 0) { - swap_node_data(list, swp, swp->next); - ++swaps; - } - } - - // if there were no swaps, the list is sorted and we may end - if (swaps == 0U) - return; - } -} diff --git a/review/list_utils.h b/review/list_utils.h deleted file mode 100644 index 2faf6476..00000000 --- a/review/list_utils.h +++ /dev/null @@ -1,64 +0,0 @@ -#ifndef LIST_UTILS_H -#define LIST_UTILS_H - -/** - * \file list_utils.h - * \author ... - */ - -#include - -#include "list.h" - -//----------------------------------------------------------------------------- -// Types -//----------------------------------------------------------------------------- - -/** - * \brief Predicate. - */ -typedef bool (*predicate)(const void *); - -/** - * \brief Comparator. - * - * Let a1 < a2 denote that argument 1 should be before argument 2 in the - * ordering implemented by this function; then the comparator returns number - *
    - *
  • < 0 if a1 < a2
    • - *
  • > 0 if a1 > a2
    • - *
  • 0 otherwise
    • - *
- */ -typedef int (*comparator)(const void *, const void *); - -/** - * \brief Action. - */ -typedef void (*action)(void *); - -//----------------------------------------------------------------------------- -// Enumeration -//----------------------------------------------------------------------------- - -/** - * \brief Calls function \p f on each element from head to tail. - * - * \param list the list - * \param f the function to call - */ -void list_for_each(struct list *list, action f); - -//----------------------------------------------------------------------------- -// Sorting -//----------------------------------------------------------------------------- - -/** - * \brief Sorts all elements in the list using \p cmp comparator. - * - * \param list the list - * \param cmp the comparator - */ -void list_sort(struct list *list, comparator cmp); - -#endif // LIST_UTILS_H diff --git a/review/main.c b/review/main.c deleted file mode 100644 index 87e7c481..00000000 --- a/review/main.c +++ /dev/null @@ -1,209 +0,0 @@ -#include -#include -#include - -#include "readline.h" -#include "list.h" -#include "list_utils.h" - -const char *versions[5] = { - "Version 1.2.5", - "Version 2.2.3", - NULL, - "Version 4.2.4", - "Version 5.2.2", -}; -const char* beta_version = "Version 6.0.0beta"; -const char* mobile_version = "Version 5.2.2mobile"; - -struct line_info -{ - int number; - char *line; - char *text; -}; - -void free_line_info(void *ptr) -{ - struct line_info *info = (struct line_info *) ptr; - free(info->line); -} - -int line_info_cmp(const void *a, const void *b) -{ - return ((struct line_info *) a)->number - ((struct line_info *) b)->number; -} - -int read_lines(struct list *list, FILE *in) -{ - char *line; - while ((line = readline(in)) != NULL) { - struct line_info info; - - info.line = line; - - char *num = strtok(line, ":"); - char *endptr; - - info.number = strtol(num, &endptr, 10); - - if (*endptr != '\0') { - fprintf(stderr, "on line %zu: invalid number '%s'\n", list_size(list) + 1, num); - return 0x00; - } - - info.text = strtok(NULL, ""); - - if (!list_push_back(list, &info)) { - perror("list_push_back"); - free_line_info(&info); - return 0x00; - } - } - - return 0x20; -} - -void write_line(void *ptr) -{ - printf("%s", ((struct line_info *) ptr)->text); -} - -int mode_a() { - printf("Mode A\n"); - - char *input_file = "input.in"; - FILE *in; - - in = fopen(input_file, "r"); - - int status = EXIT_FAILURE; - struct list list; - list_init(&list, sizeof(struct line_info)); - - if ((status = read_lines(&list, in)) == 0x20) { - list_sort(&list, &line_info_cmp); - list_for_each(&list, &write_line); - status = EXIT_SUCCESS; - } - - // ‹list_destroy()› will not clear memory correctly! - struct line_info item; - while (list_pop_front(&list, &item)) free_line_info(&item); - - list_destroy(&list); - - return status; -} - -int mode_b() { - printf("Pick a version: \n1-5 - Version 1.x.y - 5.x.y\nb - Version 5.x.yb\nm - Version 1.x.ym"); - int version = getchar(); - if ((version >= '1' && version <= '5') || version == 'm' || version == 'b') { - if (version == '1') { - printf("%s", versions[0]); - if (versions[0] == NULL) { - printf("Error"); - return EXIT_FAILURE; - } - // TODO: load the into config - } else if (version == '2') { - printf("%s", versions[1]); - if (versions[1] == NULL) { - printf("Error"); - return EXIT_FAILURE; - } - // TODO: load the into config - } else if (version == '3') { - printf("%s", versions[2]); - if (versions[2] == NULL) { - printf("Error"); - return EXIT_FAILURE; - } - // TODO: load the into config - } else if (version == '4') { - printf("%s", versions[3]); - if (versions[3] == NULL) { - printf("Error"); - return EXIT_FAILURE; - } - // TODO: load the into config - } else if (version == '5') { - printf("%s", versions[4]); - if (versions[4] == NULL) { - printf("Error"); - return EXIT_FAILURE; - } - // TODO: load the into config - } else if (version == 'b') { - printf("%s", beta_version); - // TODO: load the into config - } else if (version == 'm') { - printf("%s", mobile_version); - // TODO: load the into config - } - } else { - printf("Error"); - return EXIT_FAILURE; - } -} - -void sort_data(int data[100]) { - for (size_t i = 0; i < 100; i++) { - for (size_t j = i; i < 100; i++) { - if (data[i] > data[j]) { - int tmp = data[i]; - data[i] = data[j]; - data[j] = tmp; - } - } - } -} - -int mode_c() { - int *data = calloc(100, sizeof(int)); - if (data == NULL) { - printf("Error"); - return EXIT_FAILURE; - } - for (size_t i = 1; i <= 100; i++) { - if (i % 25 == 0) { - if (i % 5 == 0) { - data[i - 1] = 0x123; // TODO assign correct data - if (data[i - 2] == 0) { - data[i - 2] += 0x456; // TODO assign correct data - continue; - } - } else - continue; - } - if (i % 20 == 0) { - data[i - 1] = 0x456; // TODO assign correct data - if (i % 10 == 0) { - data[i - 2] += 0x789; // TODO assign correct data - continue; - } - } - if (i % 5 == 0 && !(i % 10 == 0)) { - continue; - } - } - sort_data(data); -} - -int main(int argc, char *argv[]) { - printf("Pick a mode: \na - Mode A\nb - Mode B\nc - Mode C\n"); - int mode = getchar(); - if (mode < 97 || mode > 100) { - printf("Error"); - return EXIT_FAILURE; - } - if (mode == 'a') { - mode_a(); - } else if (mode == 'b') { - mode_b(); - } else if (mode == 'c') { - mode_c(); - } - return EXIT_SUCCESS; -} diff --git a/review/readline.c b/review/readline.c deleted file mode 100644 index e35c76f2..00000000 --- a/review/readline.c +++ /dev/null @@ -1,48 +0,0 @@ -#include "readline.h" - -#include -#include -#include -#include -#include - -char *readline(FILE *file) -{ - assert(file != NULL); - size_t capacity = 0u; - size_t length = 0u; - size_t chunk_size = 64u; - size_t offset = 0u; - char *memory = NULL; - char *last_chunk; - - do { - char *tmp; - - // increase buffer size - capacity += chunk_size; - if ((tmp = realloc(memory, capacity)) == NULL) { - free(memory); - return NULL; - } - - // read next chunk - memory = tmp; - last_chunk = memory + length - offset; - - if (fgets(last_chunk, chunk_size + offset, file) == NULL) - break; - - offset = 1; - length += chunk_size; - - // found newline? - } while (strchr(last_chunk, '\n') == NULL); - - if (length == 0u || ferror(file)) { - free(memory); - return NULL; - } - - return memory; -} diff --git a/review/readline.h b/review/readline.h deleted file mode 100644 index c4a46bff..00000000 --- a/review/readline.h +++ /dev/null @@ -1,18 +0,0 @@ -#ifndef READLINE_H -#define READLINE_H - -#include - -/** - * Reads an entire line from the input. - * - * \note The caller is required to call \c free() on the returned pointer - * at some point before exiting the program. - * - * \param file input file - * \return pointer to the newly allocated memory containing the lin - * or \c NULL on failure - */ -char *readline(FILE *file); - -#endif // READLINE_H From e05a811ccaaaed7da6a65980f3248ba9782bbceb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 14:04:55 +0200 Subject: [PATCH 05/18] Changes --- lib/checkNewCronJobs.js | 4 ++-- lib/checkPullRequestJob.js | 2 +- lib/utils.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/checkNewCronJobs.js b/lib/checkNewCronJobs.js index 3cdfc20d..303719bb 100644 --- a/lib/checkNewCronJobs.js +++ b/lib/checkNewCronJobs.js @@ -17,7 +17,7 @@ const { } = require('../userWhitelist.json'); const { DATASTORE_LABEL, - JOBS_AND_FETURES_TESTING_WIKI_LINK, + JOBS_AND_FEATURES_TESTING_WIKI_LINK, getAllChangedFiles, hasDatastoreLabel, pingAndAssignUsers @@ -79,7 +79,7 @@ const getCommentBody = (changedFiles, prAuthor) => { const serverJobsForm = 'server jobs form'.link( 'https://goo.gl/forms/XIj00RJ2h5L55XzU2'); const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); + JOBS_AND_FEATURES_TESTING_WIKI_LINK); const serverAdminPing = ( 'Hi @' + serverJobAdmins.join(', @') + diff --git a/lib/checkPullRequestJob.js b/lib/checkPullRequestJob.js index 7b1fc2bb..15ec5e6d 100644 --- a/lib/checkPullRequestJob.js +++ b/lib/checkPullRequestJob.js @@ -43,7 +43,7 @@ const getCommentBody = (prAuthor) => { const serverJobsForm = 'server jobs form'.link( 'https://goo.gl/forms/XIj00RJ2h5L55XzU2'); const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); + JOBS_AND_FEATURES_TESTING_WIKI_LINK); const serverAdminPing = ( 'Hi @' + serverJobAdmins.join(', @') + diff --git a/lib/utils.js b/lib/utils.js index 7da3f697..501da296 100644 --- a/lib/utils.js +++ b/lib/utils.js @@ -22,7 +22,7 @@ const DATASTORE_LABEL = 'PR: Affects datastore layer'; const HOTFIX_LABEL = 'PR: Needs to be hotfixed'; const CODE_OWNERS_FILE_URL = 'https://raw.githubusercontent.com/oppia/oppia/develop/.github/CODEOWNERS'; -const JOBS_AND_FETURES_TESTING_WIKI_LINK = ( +const JOBS_AND_FEATURES_TESTING_WIKI_LINK = ( 'https://github.com/oppia/oppia/wiki/' + 'Testing-jobs-and-other-features-on-production' ); From 81ff2f5e0824b945b0e34a4867553534d1c8c77d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 14:21:08 +0200 Subject: [PATCH 06/18] Tests --- spec/checkPullRequestJobSpec.js | 930 ++++++++++++++++++++++++++++++++ 1 file changed, 930 insertions(+) create mode 100644 spec/checkPullRequestJobSpec.js diff --git a/spec/checkPullRequestJobSpec.js b/spec/checkPullRequestJobSpec.js new file mode 100644 index 00000000..34057040 --- /dev/null +++ b/spec/checkPullRequestJobSpec.js @@ -0,0 +1,930 @@ +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS-IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +require('dotenv').config(); +const { createProbot } = require('probot'); +// The plugin refers to the actual app in index.js. +const oppiaBot = require('../index'); +const checkPullRequestJobModule = require('../lib/checkPullRequestJob'); +const apiForSheetsModule = require('../lib/apiForSheets'); +const checkPullRequestLabelsModule = require('../lib/checkPullRequestLabels'); +const checkPullRequestBranchModule = require('../lib/checkPullRequestBranch'); +const checkCriticalPullRequestModule = + require('../lib/checkCriticalPullRequest'); +const checkPullRequestTemplateModule = + require('../lib/checkPullRequestTemplate'); +const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); +const scheduler = require('../lib/scheduler'); +const checkCronJobModule = require('../lib/checkNewCronJobs'); +const { JOBS_AND_FETURES_TESTING_WIKI_LINK } = require('../lib/utils'); + +let payloadData = JSON.parse( + JSON.stringify(require('../fixtures/pullRequestPayload.json')) +); + +describe('Pull Request Job Spec', () => { + /** + * @type {import('probot').Probot} robot + */ + let robot; + + /** + * @type {import('probot').Octokit} github + */ + let github; + + /** + * @type {import('probot').Application} app + */ + let app; + + const firstNewJobFileObj = { + sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', + filename: 'core/jobs/exp_jobs_one_off.py', + status: 'added', + additions: 1, + deletions: 0, + changes: 1, + blob_url: + 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/jobs/exp_jobs_one_off.py', + raw_url: + 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/jobs/exp_jobs_one_off.py', + contents_url: + 'https://api.github.com/repos/oppia/oppia/contents/core/jobs/' + + 'exp_jobs_one_off.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', + patch: '@@ -0,0 +1 @@\n+class FirstTestJob(jobs.JobTestBase):\n' + + '+ """One-off job for creating and populating ' + + 'UserContributionsModels for\n+ ' + + 'all registered users that have contributed.\n+ """\n+ ' + + '@classmethod\n+ def entity_classes_to_map_over(cls)' + + ':\n+ """Return a list of datastore class references ' + + 'to map over."""\n+ return [exp_models.' + + 'ExplorationSnapshotMetadataModel]\n+\n+ @staticmethod\n+ ' + + 'def map(item):\n+ """Implements the map function for this ' + + 'job."""\n+ yield (\n+ item.committer_id, ' + + '{\n+ \'exploration_id\': item.' + + 'get_unversioned_instance_id(),\n+ ' + + '\'version_string\': item.get_version_string(),\n+ ' + + '})\n+\n+\n+ @staticmethod\n+ def reduce(key, ' + + 'version_and_exp_ids):\n+ """Implements the reduce ' + + 'function for this job."""\n+ created_exploration_ids = ' + + 'set()\n+ edited_exploration_ids = set()\n+\n+ ' + + 'edits = [ast.literal_eval(v) for v in version_and_exp_ids]' + + '\n+\n+ for edit in edits:\n+ ' + + 'edited_exploration_ids.add(edit[\'exploration_id\'])' + + '\n+ if edit[\'version_string\'] == \'1\'' + + ':\n+ created_exploration_ids.add(edit' + + '[\'exploration_id\'])\n+\n+ if user_services.' + + 'get_user_contributions(key, strict=False) is not None' + + ':\n+ user_services.update_user_contributions' + + '(\n+ key, list(created_exploration_ids), ' + + 'list(\n+ edited_exploration_ids))\n+ ' + + 'else:\n+ user_services.create_user_contributions' + + '(\n+ key, list(created_exploration_ids), list' + + '(\n+ edited_exploration_ids))\n', + }; + + const secondNewJobFileObj = { + sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', + filename: 'core/jobs/exp_jobs_oppiabot_off.py', + status: 'added', + additions: 1, + deletions: 0, + changes: 1, + blob_url: + 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', + raw_url: + 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', + contents_url: + 'https://api.github.com/repos/oppia/oppia/contents/core/jobs/' + + 'exp_jobs_oppiabot_off.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', + patch: '@@ -0,0 +1 @@\n+class SecondTestJob(jobs.JobTestBase):\n' + + '+ """One-off job for creating and populating ' + + 'UserContributionsModels for\n' + + '+ all registered users that have contributed.\n+ ' + + '"""\n+ @classmethod\n+ def entity_classes_to_map_over' + + '(cls):\n+ """Return a list of datastore class references ' + + 'to map over."""\n+ return [exp_models.' + + 'ExplorationSnapshotMetadataModel]\n+\n+ @staticmethod\n+ ' + + 'def map(item):\n+ """Implements the map function for this ' + + 'job."""\n+ yield (\n+ item.committer_id, ' + + '{\n+ \'exploration_id\': item.' + + 'get_unversioned_instance_id(),\n+ \'version_string\'' + + ': item.get_version_string(),\n+ })\n+\n+\n+ ' + + '@staticmethod\n+ def reduce(key, version_and_exp_ids)' + + ':\n+ """Implements the reduce function for this job.' + + '"""\n+ created_exploration_ids = set()\n+ ' + + 'edited_exploration_ids = set()\n+\n+ edits = ' + + '[ast.literal_eval(v) for v in version_and_exp_ids]\n+\n+ ' + + 'for edit in edits:\n+ edited_exploration_ids.add' + + '(edit[\'exploration_id\'])\n+ if edit' + + '[\'version_string\'] == \'1\':\n+ ' + + 'created_exploration_ids.add(edit[\'exploration_id\'])' + + '\n+\n+ if user_services.get_user_contributions' + + '(key, strict=False) is not None:\n+ user_services.' + + 'update_user_contributions(\n+ key, list' + + '(created_exploration_ids), list(\n+ ' + + 'edited_exploration_ids))\n+ else:\n+ ' + + 'user_services.create_user_contributions(\n+ ' + + 'key, list(created_exploration_ids), list(\n+ ' + + 'edited_exploration_ids))\n', + }; + + const modifiedExistingJobFileObj = { + sha: 'f06a0d3ea104733080c4dad4a4e5aa7fb76d8f5d', + filename: 'core/jobs/user_jobs_one_off.py', + status: 'modified', + additions: 43, + deletions: 0, + changes: 43, + blob_url: + 'https://github.com/oppia/oppia/blob/5b0e633fa1b9a00771a3b88302fa' + + '3ff048d7240c/core/jobs/user_jobs_one_off.py', + raw_url: + 'https://github.com/oppia/oppia/raw/5b0e633fa1b9a00771a3b88302fa' + + '3ff048d7240c/core/jobs/user_jobs_one_off.py', + contents_url: + 'https://api.github.com/repos/oppia/oppia/contents/core/jobs/' + + 'user_jobs_one_off.py?ref=5b0e633fa1b9a00771a3b88302fa3ff048d7240c', + patch: + '@@ -80,6 +80,49 @@ def reduce(key, version_and_exp_ids)' + + ':\n edited_exploration_ids))\n \n ' + + '\n+class OppiabotContributionsJob(jobs.JobTestBase):\n' + + '+ """One-off job for creating and populating ' + + 'UserContributionsModels for' + + '\n+ all registered users that have contributed.\n+ ' + + '"""\n+ @classmethod\n+ def entity_classes_to_map_over' + + '(cls):\n+ """Return a list of datastore class ' + + 'references to map over."""\n+ return ' + + '[exp_models.ExplorationSnapshotMetadataModel]\n+\n+ ' + + '@staticmethod\n+ def map(item):\n+ """Implements ' + + 'the map function for this job."""\n+ yield ' + + '(\n+ item.committer_id, {\n+ ' + + '\'exploration_id\': item.get_unversioned_instance_id(),' + + '\n+ \'version_string\': item.' + + 'get_version_string(),\n+ })\n+\n+\n+ ' + + '@staticmethod\n+ def reduce(key, version_and_exp_ids)' + + ':\n+ """Implements the reduce function for this ' + + 'job."""\n+ created_exploration_ids = set()' + + '\n+ edited_exploration_ids = set()\n+\n+ ' + + 'edits = [ast.literal_eval(v) for v in version_and_exp_ids]' + + '\n+\n+ for edit in edits:\n+ ' + + 'edited_exploration_ids.add(edit[\'exploration_id\'])' + + '\n+ if edit[\'version_string\'] == \'1\'' + + ':\n+ created_exploration_ids.add(edit' + + '[\'exploration_id\'])\n+\n+ if user_services.' + + 'get_user_contributions(key, strict=False) is not None' + + ':\n+ user_services.update_user_contributions' + + '(\n+ key, list(created_exploration_ids), ' + + 'list(\n+ edited_exploration_ids))' + + '\n+ else:\n+ user_services.' + + 'create_user_contributions(\n+ key, ' + + 'list(created_exploration_ids), list(' + + '\n+ edited_exploration_ids))\n', + }; + const modifiedExistingJobFileObjWithJobClassInPatch = { + sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', + filename: 'core/jobs/exp_jobs_oppiabot_off.py', + status: 'modified', + additions: 1, + deletions: 0, + changes: 1, + blob_url: + 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', + raw_url: + 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', + contents_url: + 'https://api.github.com/repos/oppia/oppia/contents/core/jobs/' + + 'exp_jobs_oppiabot_off.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', + patch: '@@ -0,0 +1 @@class SecondTestJob(jobs.JobTestBase) ' + + 'def reduce(key, version_and_exp_ids):\n' + + ' edited_exploration_ids))\n ', + }; + + const jobFileObjWithJobClassInPatchAndNewJob = { + sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', + filename: 'core/jobs/exp_jobs_oppiabot_off.py', + status: 'modified', + additions: 1, + deletions: 0, + changes: 1, + blob_url: + 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', + raw_url: + 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a89413' + + '0e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', + contents_url: + 'https://api.github.com/repos/oppia/oppia/contents/core/jobs/' + + 'exp_jobs_oppiabot_off.py?ref=' + + '67fb4a973b318882af3b5a894130e110d7e9833c', + patch: '@@ -0,0 +1 @@class SecondTestJob(jobs.JobTestBase) ' + + 'def reduce(key, version_and_exp_ids):\n' + + ' edited_exploration_ids))\n' + + ' \n' + + '+class AnotherTestJob(jobs.JobTestBase):\n' + + '+ """\n' + + '+ @classmethod\n' + + '+ def entity_classes_to_map_over ', + }; + + const jobFromTestDir = { + sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', + filename: 'core/tests/linter_tests/exp_jobs_oppiabot_off.py', + status: 'added', + additions: 1, + deletions: 0, + changes: 1, + blob_url: + 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/tests/linter_tests/exp_jobs_oppiabot_off.py', + raw_url: + 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/tests/linter_tests/exp_jobs_oppiabot_off.py', + contents_url: + 'https://api.github.com/repos/oppia/oppia/contents/core/tests/' + + 'linter_tests/exp_jobs_oppiabot_off.py?ref=' + + '67fb4a973b318882af3b5a894130e110d7e9833c', + patch: '@@ -0,0 +1 @@\n+class LintTestJob(jobs.JobTestBase):\n' + + '+ """One-off job for creating and populating' + + ' UserContributionsModels for\n' + + '+ all registered users that have contributed.\n+ ' + + '"""\n+ @classmethod\n+ def entity_classes_to_map_over' + + '(cls):\n+ """Return a list of datastore class ' + + 'references to map over."""\n+ return [exp_models.' + + 'ExplorationSnapshotMetadataModel]\n+\n+ @staticmethod' + + '\n+ def map(item):\n+ """Implements the map function ' + + 'for this job."""\n+ yield (\n+ item.' + + 'committer_id, {\n+ \'exploration_id\': ' + + 'item.get_unversioned_instance_id(),\n+ ' + + '\'version_string\': item.get_version_string(),\n+ ' + + '})\n+\n+\n+ @staticmethod\n+ def reduce(key, ' + + 'version_and_exp_ids):\n+ """Implements the reduce ' + + 'function for this job."""\n+ created_exploration_ids ' + + '= set()\n+ edited_exploration_ids = set()\n+\n+ ' + + 'edits = [ast.literal_eval(v) for v in version_and_exp_ids]' + + '\n+\n+ for edit in edits:\n+ ' + + 'edited_exploration_ids.add(edit[\'exploration_id\'])' + + '\n+ if edit[\'version_string\'] == \'1\'' + + ':\n+ created_exploration_ids.add(edit' + + '[\'exploration_id\'])\n+\n+ if user_services.' + + 'get_user_contributions(key, strict=False) is not None' + + ':\n+ user_services.update_user_contributions' + + '(\n+ key, list(created_exploration_ids), ' + + 'list(\n+ edited_exploration_ids))\n+ ' + + 'else:\n+ user_services.create_user_contributions' + + '(\n+ key, list(created_exploration_ids), list' + + '(\n+ edited_exploration_ids))\n', + }; + + const fileWithMultipleJobs = { + sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', + filename: 'core/jobs/exp_jobs_oppiabot_off.py', + status: 'added', + additions: 1, + deletions: 0, + changes: 1, + blob_url: + 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', + raw_url: + 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', + contents_url: + 'https://api.github.com/repos/oppia/oppia/contents/core/jobs/' + + 'exp_jobs_oppiabot_off.py?ref=' + + '67fb4a973b318882af3b5a894130e110d7e9833c', + patch: '@@ -0,0 +1 @@\n+class TestJob(jobs.JobTestBase):\n' + + '+ """One-off job for creating and ' + + 'populating UserContributionsModels for \n+class ' + + 'AnotherTestJob(jobs.JobTestBase):\n' + + '+ """\n+ @classmethod\n+ def ' + + 'entity_classes_to_map_over(cls):\n+ """Return a list' + + ' of datastore class references to map over."""\n+ ' + + 'return [exp_models.ExplorationSnapshotMetadataModel]\n+\n+ ' + + '@staticmethod\n+ def map(item):\n+ """Implements ' + + 'the map function for this job."""\n+ yield ' + + '(\n+ item.committer_id, {\n+ ' + + '\'exploration_id\': item.get_unversioned_instance_id(),\n+' + + ' \'version_string\': item.get_version_string(),' + + '\n+ })\n+\n+\n+ @staticmethod\n+ ' + + 'def reduce(key, version_and_exp_ids):\n+ ' + + '"""Implements the reduce function for this job."""\n+ ' + + 'created_exploration_ids = set()\n+ ' + + 'edited_exploration_ids = set()\n+\n+ edits = ' + + '[ast.literal_eval(v) for v in version_and_exp_ids]' + + '\n+\n+ for edit in edits:\n+ ' + + 'edited_exploration_ids.add(edit[\'exploration_id\'])' + + '\n+ if edit[\'version_string\'] == \'1\':\n+' + + ' created_exploration_ids.add(edit' + + '[\'exploration_id\'])\n+\n+ if user_services.' + + 'get_user_contributions(key, strict=False) is not None' + + ':\n+ user_services.update_user_contributions' + + '(\n+ key, list(created_exploration_ids), ' + + 'list(\n+ edited_exploration_ids))' + + '\n+ else:\n+ user_services.' + + 'create_user_contributions(\n+ key, list(' + + 'created_exploration_ids), list(\n+ ' + + 'edited_exploration_ids))\n', + }; + + const jobTestFile = { + sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', + filename: 'core/jobs/exp_jobs_oppiabot_off_test.py', + status: 'added', + additions: 1, + deletions: 0, + changes: 1, + blob_url: + 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', + raw_url: + 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/jobs/exp_jobs_oppiabot_off.py', + contents_url: + 'https://api.github.com/repos/oppia/oppia/contents/core/jobs/' + + 'exp_jobs_oppiabot_off.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', + patch: '@@ -0,0 +1 @@\n+class TestJobTests(jobs.JobTestBase):\n' + + '+ """One-off job for creating and populating ' + + 'UserContributionsModels for \n' + + '+class AnotherTestJobTests(jobs.JobTestBase):\n' + + '+ """\n+ ' + + '@classmethod\n+ def entity_classes_to_map_over(cls):' + + '\n+ """Return a list of datastore class references to ' + + 'map over."""\n+ return [exp_models.' + + 'ExplorationSnapshotMetadataModel]\n+\n+ @staticmethod\n+ ' + + 'def map(item):\n+ """Implements the map function for this ' + + 'job."""\n+ yield (\n+ item.committer_id, ' + + '{\n+ \'exploration_id\': item.get_unversioned_' + + 'instance_id(),\n+ \'version_string\': item.' + + 'get_version_string(),\n+ })\n+\n+\n+ ' + + '@staticmethod\n+ def reduce(key, version_and_exp_ids):\n+' + + ' """Implements the reduce function for this job."""\n+' + + ' created_exploration_ids = set()\n+ ' + + 'edited_exploration_ids = set()\n+\n+ edits ' + + '= [ast.literal_eval(v) for v in version_and_exp_ids]\n+\n+ ' + + 'for edit in edits:\n+ edited_exploration_ids.add(edit' + + '[\'exploration_id\'])\n+ if edit[\'version_string\'] == ' + + '\'1\':\n+ created_exploration_ids.add(edit' + + '[\'exploration_id\'])\n+\n+ if user_services.' + + 'get_user_contributions(key, strict=False) is not None:\n+ ' + + 'user_services.update_user_contributions(\n+ key, list' + + '(created_exploration_ids), list(\n+ ' + + 'edited_exploration_ids))\n+ else:\n+ ' + + 'user_services.create_user_contributions(\n+ ' + + 'key, list(created_exploration_ids), list(\n+ ' + + 'edited_exploration_ids))\n', + }; + + const nonJobFile = { + sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', + filename: 'core/domain/exp_fetchers.py', + status: 'added', + additions: 1, + deletions: 0, + changes: 1, + blob_url: + 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/domain/exp_fetchers.py', + raw_url: + 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/domain/exp_fetchers.py', + contents_url: + 'https://api.github.com/repos/oppia/oppia/contents/core/domain/' + + 'exp_fetchers.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', + patch: '@@ -0,0 +1 @@\n+# def _migrate_states_schema' + + '(versioned_exploration_states, exploration_id):', + }; + + beforeEach(() => { + spyOn(scheduler, 'createScheduler').and.callFake(() => { }); + + github = { + issues: { + createComment: jasmine.createSpy('createComment').and.returnValue({}), + addLabels: jasmine.createSpy('addLabels').and.returnValue({}), + addAssignees: jasmine.createSpy('addAssignees').and.returnValue({}) + }, + }; + + robot = createProbot({ + id: 1, + cert: 'test', + githubToken: 'test', + }); + + app = robot.load(oppiaBot); + spyOn(app, 'auth').and.resolveTo(github); + spyOn(checkPullRequestJobModule, 'checkForNewJob').and.callThrough(); + spyOn(apiForSheetsModule, 'checkClaStatus').and.callFake(() => { }); + spyOn( + checkCriticalPullRequestModule, 'checkIfPRAffectsDatastoreLayer' + ).and.callFake(() => { }); + spyOn(checkPullRequestBranchModule, 'checkBranch').and.callFake(() => { }); + spyOn(checkCronJobModule, 'checkForNewCronJob').and.callFake(() => { }); + spyOn( + checkPullRequestTemplateModule, 'checkTemplate' + ).and.callFake(() => { }); + spyOn(newCodeOwnerModule, 'checkForNewCodeowner').and.callFake(() => { }); + }); + + describe('When a new job file is created in a pull request', () => { + beforeEach(async () => { + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [nonJobFile, firstNewJobFileObj], + }), + }; + payloadData.payload.pull_request.changed_files = 2; + await robot.receive(payloadData); + }); + + it('should check for jobs', () => { + expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + }); + + it('should get modified files', () => { + expect(github.pulls.listFiles).toHaveBeenCalled(); + }); + + it('should ping server jobs admin', () => { + const author = payloadData.payload.pull_request.user.login; + const formText = ( + 'server jobs form'.link( + 'https://goo.gl/forms/XIj00RJ2h5L55XzU2') + ); + const newLineFeed = '
'; + const wikiLinkText = 'this guide'.link( + JOBS_AND_FETURES_TESTING_WIKI_LINK); + const jobNameLink = ( + 'FirstTestJob'.link(firstNewJobFileObj.blob_url) + ); + + expect(github.issues.createComment).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + body: + 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, ' + + 'it adds a new job. The name of the job is ' + jobNameLink + '.' + + newLineFeed + + 'Also @' + author + ', please make sure to fill in the ' + formText + + ' for the new job to be tested on the backup server. ' + + 'This PR can be merged only after the test run is successful. ' + + 'Please refer to ' + wikiLinkText + ' for details.' + + newLineFeed + 'Thanks!', + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + }); + }); + + it('should assign server jobs admin', () => { + expect(github.issues.addAssignees).toHaveBeenCalled(); + expect(github.issues.addAssignees).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + assignees: ['vojtechjelinek', 'DubeySandeep', 'kevintab95'] + }); + }); + + it('should add datastore label', () => { + expect(github.issues.addLabels).toHaveBeenCalled(); + expect(github.issues.addLabels).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + labels: ['PR: Affects datastore layer'] + }); + }); + }); + + describe('When multiple job files are created in a pull request', () => { + beforeEach(async () => { + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [ + nonJobFile, + firstNewJobFileObj, + secondNewJobFileObj + ], + }), + }; + + payloadData.payload.pull_request.changed_files = 3; + await robot.receive(payloadData); + }); + + it('should check for jobs', () => { + expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + }); + + it('should get modified files', () => { + expect(github.pulls.listFiles).toHaveBeenCalled(); + }); + + it('should ping server jobs admin', () => { + expect(github.issues.createComment).toHaveBeenCalled(); + const author = payloadData.payload.pull_request.user.login; + const formText = ( + 'server jobs form'.link('https://goo.gl/forms/XIj00RJ2h5L55XzU2') + ); + const newLineFeed = '
'; + const wikiLinkText = 'this guide'.link( + JOBS_AND_FETURES_TESTING_WIKI_LINK); + const jobRegistryLink = ( + 'job registry'.link( + 'https://github.com/oppia/oppia/blob/develop/core/jobs_registry.py') + ); + const firstJobNameLink = ( + 'FirstTestJob'.link(firstNewJobFileObj.blob_url) + ); + const secondJobNameLink = ( + 'SecondTestJob'.link(secondNewJobFileObj.blob_url) + ); + expect(github.issues.createComment).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + body: + 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, ' + + 'it adds new jobs. The jobs are ' + firstJobNameLink + + ', ' + secondJobNameLink + '.' + newLineFeed + 'Also @' + + author + ', please make sure to fill in the ' + + formText + ' for the new jobs to be tested on the backup server. ' + + 'This PR can be merged only after the test run is successful. ' + + 'Please refer to ' + wikiLinkText + ' for details.' + + newLineFeed + 'Thanks!', + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + }); + }); + + it('should assign server jobs admin', () => { + expect(github.issues.addAssignees).toHaveBeenCalled(); + expect(github.issues.addAssignees).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + assignees: ['vojtechjelinek', 'DubeySandeep', 'kevintab95'] + }); + }); + + it('should add datastore label', () => { + expect(github.issues.addLabels).toHaveBeenCalled(); + expect(github.issues.addLabels).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + labels: ['PR: Affects datastore layer'] + }); + }); + }); + + describe('When a new job file is created and registry is ' + + 'updated in a pull request', + () => { + beforeEach(async () => { + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [ + nonJobFile, + firstNewJobFileObj + ], + }), + }; + payloadData.payload.pull_request.changed_files = 2; + await robot.receive(payloadData); + }); + + it('should check for jobs', () => { + expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + }); + + it('should get modified files', () => { + expect(github.pulls.listFiles).toHaveBeenCalled(); + }); + + it('should ping server jobs admin', () => { + expect(github.issues.createComment).toHaveBeenCalled(); + const author = payloadData.payload.pull_request.user.login; + const formText = ( + 'server jobs form'.link('https://goo.gl/forms/XIj00RJ2h5L55XzU2') + ); + const newLineFeed = '
'; + const wikiLinkText = 'this guide'.link( + JOBS_AND_FETURES_TESTING_WIKI_LINK); + const jobNameLink = ( + 'FirstTestJob'.link(firstNewJobFileObj.blob_url) + ); + expect(github.issues.createComment).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + body: + 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this ' + + 'PR, it adds a new job. The name of the job is ' + jobNameLink + + '.' + newLineFeed + 'Also @' + author + + ', please make sure to fill in the ' + formText + + ' for the new job to be tested on the backup server. ' + + 'This PR can be merged only after the test run is successful. ' + + 'Please refer to ' + wikiLinkText + ' for details.' + + newLineFeed + 'Thanks!', + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + }); + }); + + it('should assign server jobs admin', () => { + expect(github.issues.addAssignees).toHaveBeenCalled(); + expect(github.issues.addAssignees).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + assignees: ['vojtechjelinek', 'DubeySandeep', 'kevintab95'] + }); + }); + + it('should add datastore label', () => { + expect(github.issues.addLabels).toHaveBeenCalled(); + expect(github.issues.addLabels).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + labels: ['PR: Affects datastore layer'] + }); + }); + } + ); + + describe('When a new job is added in an existing job file', () => { + beforeEach(async () => { + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [ + modifiedExistingJobFileObj, + ], + }), + }; + payloadData.payload.pull_request.changed_files = 1; + await robot.receive(payloadData); + }); + + it('should check for jobs', () => { + expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + }); + + it('should get modified files', () => { + expect(github.pulls.listFiles).toHaveBeenCalled(); + }); + + + it('should ping server jobs admin', () => { + expect(github.issues.createComment).toHaveBeenCalled(); + const author = payloadData.payload.pull_request.user.login; + const formText = ( + 'server jobs form'.link('https://goo.gl/forms/XIj00RJ2h5L55XzU2') + ); + const newLineFeed = '
'; + const wikiLinkText = 'this guide'.link( + JOBS_AND_FETURES_TESTING_WIKI_LINK); + const jobRegistryLink = ( + 'job registry'.link( + 'https://github.com/oppia/oppia/blob/develop/core/jobs_registry.py') + ); + const jobNameLink = ( + 'OppiabotContributionsJob' + .link(modifiedExistingJobFileObj.blob_url) + ); + expect(github.issues.createComment).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + body: + 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, ' + + 'it adds a new job. The name of the job is ' + jobNameLink + + '.' + newLineFeed + 'Also @' + author + ', please make sure to ' + + 'fill in the ' + formText + ' for the new job to be tested on the ' + + 'backup server. ' + 'This PR can be merged only after the test ' + + 'run is successful. ' + 'Please refer to ' + wikiLinkText + + ' for details.' + newLineFeed + 'Thanks!', + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + }); + }); + + it('should assign server jobs admin', () => { + expect(github.issues.addAssignees).toHaveBeenCalled(); + expect(github.issues.addAssignees).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + assignees: ['vojtechjelinek', 'DubeySandeep', 'kevintab95'] + }); + }); + + it('should add datastore label', () => { + expect(github.issues.addLabels).toHaveBeenCalled(); + expect(github.issues.addLabels).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + labels: ['PR: Affects datastore layer'] + }); + }); + }); + + describe('When an existing job file is modified with no new job', () => { + beforeEach(async () => { + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [ + { + ...firstNewJobFileObj, + status: 'modified', + patch: '\n+# No job files present in the changes', + }, + ], + }), + }; + + payloadData.payload.pull_request.changed_files = 1; + await robot.receive(payloadData); + }); + + it('should check for jobs', () => { + expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + }); + + it('should get modified files', () => { + expect(github.pulls.listFiles).toHaveBeenCalled(); + }); + + + it('should not ping server jobs admin', () => { + expect(github.issues.createComment).not.toHaveBeenCalled(); + }); + it('should not add datastore label', () => { + expect(github.issues.addLabels).not.toHaveBeenCalled(); + }); + it('should not assign server jobs admin', () => { + expect(github.issues.addAssignees).not.toHaveBeenCalled(); + }); + }); + + describe('When no job file is modified in a pull request', () => { + beforeEach(async () => { + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [ + nonJobFile + ], + }), + }; + + payloadData.payload.pull_request.changed_files = 1; + await robot.receive(payloadData); + }); + + it('should check for jobs', () => { + expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + }); + + it('should not get modified files', () => { + expect(github.pulls.listFiles).toHaveBeenCalled(); + }); + + it('should not ping server job admin', () => { + expect(github.issues.createComment).not.toHaveBeenCalled(); + }); + }); + + describe('Does not comment on job from test dir', () => { + beforeEach(async () => { + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [ + jobFromTestDir + ], + }), + }; + + payloadData.payload.pull_request.changed_files = 1; + await robot.receive(payloadData); + }); + + it('should check for jobs', () => { + expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + }); + + it('should get modified files', () => { + expect(github.pulls.listFiles).toHaveBeenCalled(); + }); + + it('should not ping server job admin', () => { + expect(github.issues.createComment).not.toHaveBeenCalled(); + }); + }); + + describe('When job test file gets added', () => { + beforeEach(async () => { + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [ + jobTestFile + ], + }), + }; + + payloadData.payload.pull_request.changed_files = 1; + await robot.receive(payloadData); + }); + + it('should check for jobs', () => { + expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + }); + + it('should get modified files', () => { + expect(github.pulls.listFiles).toHaveBeenCalled(); + }); + + it('should not ping server job admin', () => { + expect(github.issues.createComment).not.toHaveBeenCalled(); + }); + }); + + describe('When pull request has datastore label', () => { + beforeEach(async () => { + payloadData.payload.pull_request.labels = [{ + name: 'PR: Affects datastore layer' + }]; + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [ + nonJobFile, firstNewJobFileObj + ], + }), + }; + + payloadData.payload.pull_request.changed_files = 2; + await robot.receive(payloadData); + }); + + it('should check for jobs', () => { + expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + }); + + it('should not get modified files', () => { + expect(github.pulls.listFiles).not.toHaveBeenCalled(); + }); + + it('should not ping server job admin', () => { + expect(github.issues.createComment).not.toHaveBeenCalled(); + }); + }); + + describe('Returns appropriate job name', () => { + it('should return the correct job created in the file', () => { + let jobs = checkPullRequestJobModule.getNewJobsFromFile( + firstNewJobFileObj + ); + expect(jobs.length).toBe(1); + expect(jobs[0]).toBe('FirstTestJob'); + + jobs = checkPullRequestJobModule.getNewJobsFromFile(secondNewJobFileObj); + expect(jobs.length).toBe(1); + expect(jobs[0]).toBe('SecondTestJob'); + + jobs = checkPullRequestJobModule.getNewJobsFromFile( + modifiedExistingJobFileObj + ); + expect(jobs.length).toBe(1); + expect(jobs[0]).toBe('OppiabotContributionsJob'); + + jobs = checkPullRequestJobModule.getNewJobsFromFile(fileWithMultipleJobs); + expect(jobs.length).toBe(2); + expect(jobs[0]).toBe('TestJob'); + expect(jobs[1]).toBe('AnotherTestJob'); + + jobs = checkPullRequestJobModule.getNewJobsFromFile(jobTestFile); + expect(jobs.length).toBe(0); + + jobs = checkPullRequestJobModule.getNewJobsFromFile( + modifiedExistingJobFileObjWithJobClassInPatch); + expect(jobs.length).toBe(0); + + jobs = checkPullRequestJobModule.getNewJobsFromFile( + jobFileObjWithJobClassInPatchAndNewJob); + expect(jobs.length).toBe(1); + expect(jobs[0]).toBe('AnotherTestJob'); + }); + }); +}); \ No newline at end of file From f34cb7fa739248d527e38cf91b35a7c1faf76344 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 14:27:31 +0200 Subject: [PATCH 07/18] Tests --- spec/checkNewCronJobSpec.js | 520 ++++++++++++++++++++++++++++++++++++ 1 file changed, 520 insertions(+) create mode 100644 spec/checkNewCronJobSpec.js diff --git a/spec/checkNewCronJobSpec.js b/spec/checkNewCronJobSpec.js new file mode 100644 index 00000000..998920c0 --- /dev/null +++ b/spec/checkNewCronJobSpec.js @@ -0,0 +1,520 @@ +// Copyright 2021 The Oppia Authors. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS-IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + + +require('dotenv').config(); +const { createProbot } = require('probot'); +// The plugin refers to the actual app in index.js. +const oppiaBot = require('../index'); +const checkCronJobModule = require('../lib/checkNewCronJobs'); +const checkPullRequestJobModule = require('../lib/checkPullRequestJob'); +const apiForSheetsModule = require('../lib/apiForSheets'); +const checkPullRequestLabelsModule = require('../lib/checkPullRequestLabels'); +const checkPullRequestBranchModule = require('../lib/checkPullRequestBranch'); +const checkCriticalPullRequestModule = + require('../lib/checkCriticalPullRequest'); +const checkPullRequestTemplateModule = + require('../lib/checkPullRequestTemplate'); +const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); +const scheduler = require('../lib/scheduler'); +const { JOBS_AND_FETURES_TESTING_WIKI_LINK } = require('../lib/utils'); +let payloadData = JSON.parse( + JSON.stringify(require('../fixtures/pullRequestPayload.json')) +); + +describe('Cron Job Spec', () => { + /** + * @type {import('probot').Probot} robot + */ + let robot; + + /** + * @type {import('probot').Octokit} github + */ + let github; + + /** + * @type {import('probot').Application} app + */ + let app; + + const firstNewJobFileObj = { + sha: 'bbc41f1bd8b2c90c91a1335d149be7a132a732d8', + filename: 'core/controllers/cron.py', + status: 'added', + additions: 37, + deletions: 0, + changes: 37, + blob_url: + 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/cron.py', + raw_url: + 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/domain/cron.py', + contents_url: + 'https://api.github.com/repos/oppia/oppia/contents/core/' + + 'cron.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', + patch: '@@ -0,0 +1,37 @@\n+class JobStatusMailerHandler(base.BaseHanr):' + + '\n+ """Handler for mailing admin about job failures."""\n+\n+' + + ' @acl_decorators.can_perform_cron_tasks\n+ def get(self):\n+' + + ' """Handles GET requests."""\n+ # TODO(sll): Get the 50 most' + + 'recent failed shards, not all of them.\n+ failed_jobs' + + ' = cron_services.get_stuck_jobs(TWENTY_FIVE_HOURS_IN_MSECS)\n+' + + ' if failed_jobs:\n+ email_subject = \'MapReduce failure' + + ' alert\'\n+ email_message = (\n+ \'%s jobs' + + ' have failed in the past 25 hours. More information \'\n+ ' + + ' \'(about at most %s jobs; to see more, please check the logs):\'\n+' + + ' ) % (len(failed_jobs), MAX_JOBS_TO_REPORT_ON)\n+\n+' + + ' for job in failed_jobs[:MAX_JOBS_TO_REPORT_ON]:\n+' + + ' email_message += \'\\n\'\n+ ' + + 'email_message += \'-----------------------------------\'\n+ ' + + ' email_message += \'\\n\'\n+ ' + + ' email_message += (\n+ \'Job with mapreduce ID %s' + + '(key name %s) failed. \'\n+ \'More info:\\n\\n\'\n+' + }; + + const urlJobFileObj = { + sha: 'f06a0d3ea104733080c4dad4a4e5aa7fb76d8f5d', + filename: 'main_cron.py', + status: 'modified', + additions: 43, + deletions: 0, + changes: 43, + blob_url: + 'https://github.com/oppia/oppia/blob/5b0e633fa1b9a00771a3b88302fa' + + '3ff048d7240c/main_cron.py', + raw_url: + 'https://github.com/oppia/oppia/raw/5b0e633fa1b9a00771a3b88302fa' + + '3ff048d7240c/main_cron.py', + contents_url: + 'https://api.github.com/repos/oppia/oppia/contents/' + + 'main_cron.py?ref=5b0e633fa1b9a00771a3b88302fa3ff048d7240c', + patch: + '@@ -0,0 +1,46 @@\n+"""Main package for URL routing and the index' + + 'page."""\n+\n+from __future__ import absolute_import # pylint:' + + ' disable=import-only-modules\n+from __future__' + + ' import unicode_literals # pylint: disable=import-only-modules\n' + + '+\n+from core.controllers import cron\n+from core.platform import' + + ' n+\n+\n+transaction_services = models.Registry.' + + ' import_transaction_services()\n+\n+# Register the URLs with' + + 'the classes responsible for handling them.\n+URLS = [\n+ ' + + 'main.get_redirect_route(\n+ r\'/cron/mail/admin/job_s' + + 'tatus\', cron.JobStatusMailerHandler),\n+' + + 'main.get_redirect_route(\n+ ' + + 'r\'/cron/users/dashboard_stats\', cron.CronDashboardStatsHandler),\n+' + + ' main.get_redirect_route(\n+ r\'/cron/users/user_deletion' + + 'r\'/cron/users/fully_complete_user_deletion\',\n+' + + ' cron.CronFullyCompleteUserDeletionHandler),\n+' + + ' main.get_redirect_route(\n+tionRecommendationsHandler),\n+' + + 'main.get_redirect_route(\n+ r\'/cron/explorations/search_rank\',\n+' + + 'cron.CronActivitySearchRankHandler),\n+ main.get_redirect_route(\n' + + ' main.get_redirect_route(\n+ r\'/cron/models/cleanup\', cron' + }; + + const jobTestFile = { + sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', + filename: 'core/controllers/cron_test.py', + status: 'added', + additions: 1, + deletions: 0, + changes: 1, + blob_url: + 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/cpntrollers/cron_test.py', + raw_url: + 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/domain/cron_test.py', + contents_url: + 'https://api.github.com/repos/oppia/oppia/contents/core/domain/' + + 'exp_jobs_oppiabot_off.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', + patch: + '@@ -0,0 +1,25 @@\n+class CronJobTests(test_utils.GenericTestBase):\n+' + + 'FIVE_WEEKS = datetime.timedelta(weeks=5)\n+ NINE_WEEKS' + + '= datetime.timedelta(weeks=9)\n+\n+ def setUp(self):\n+' + + ' super(CronJobTests, self).setUp()\n+ ' + + ' self.signup(self.ADMIN_EMAIL, self.ADMIN_USERNAME)\n+' + + 'self.admin_id = self.get_user_id_from_email(self.ADMIN_EMAIL)\n+' + + 'self.set_admins([self.ADMIN_USERNAME])\n+' + + ' self.testapp_swap = self.swap(\n+' + + 'self, \'testapp\', webtest.TestApp(main_cron.app))\n+\n+' + + 'self.email_subjects = []\n+ self.email_bodies = []\n+' + + 'def _mock_send_mail_to_admin(email_subject, email_body):\n+ ' + + 'ocks email_manager.send_mail_to_admin() as it\'s not possible to\n+' + + 'send mail with self.testapp_swap, i.e with the URLs defined in\n+' + + ' main_cron.\n+ """\n+ self.email_subjects' + + '.append(email_subject)\n+ self.email_bodies.append' + + '(email_body)\n+\n+ self.send_mail_to_admin_swap = self.swap' + + '(\n+ email_manager, \'send_mail_to_admin\',' + + '_mock_send_mail_to_admin)\n' + }; + + const nonJobFile = { + sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', + filename: 'core/domain/exp_fetchers.py', + status: 'added', + additions: 1, + deletions: 0, + changes: 1, + blob_url: + 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/domain/exp_fetchers.py', + raw_url: + 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130' + + 'e110d7e9833c/core/domain/exp_fetchers.py', + contents_url: + 'https://api.github.com/repos/oppia/oppia/contents/core/domain/' + + 'exp_fetchers.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', + patch: '@@ -0,0 +1 @@\n+# def _migrate_states_schema' + + '(versioned_exploration_states, exploration_id):', + }; + + beforeEach(() => { + spyOn(scheduler, 'createScheduler').and.callFake(() => { }); + + github = { + issues: { + createComment: jasmine.createSpy('createComment').and.returnValue({}), + addLabels: jasmine.createSpy('addLabels').and.returnValue({}), + addAssignees: jasmine.createSpy('addAssignees').and.returnValue({}) + }, + }; + + robot = createProbot({ + id: 1, + cert: 'test', + githubToken: 'test', + }); + + app = robot.load(oppiaBot); + spyOn(app, 'auth').and.resolveTo(github); + spyOn(checkCronJobModule, 'checkForNewCronJob').and.callThrough(); + spyOn( + checkPullRequestJobModule, 'checkForNewJob').and.callFake(() => { }); + spyOn(apiForSheetsModule, 'checkClaStatus').and.callFake(() => { }); + spyOn( + checkCriticalPullRequestModule, 'checkIfPRAffectsDatastoreLayer' + ).and.callFake(() => { }); + spyOn(checkPullRequestBranchModule, 'checkBranch').and.callFake(() => { }); + spyOn( + checkPullRequestTemplateModule, 'checkTemplate' + ).and.callFake(() => { }); + spyOn(newCodeOwnerModule, 'checkForNewCodeowner').and.callFake(() => { }); + }); + + describe('When a new cron job is added in a pull request', () => { + beforeEach(async () => { + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [ + nonJobFile, firstNewJobFileObj + ], + }), + }; + payloadData.payload.pull_request.changed_files = 2; + await robot.receive(payloadData); + }); + + it('should check for cron jobs', () => { + expect(checkCronJobModule.checkForNewCronJob).toHaveBeenCalled(); + }); + + it('should get modified files', () => { + expect(github.pulls.listFiles).toHaveBeenCalled(); + }); + + it('should ping server jobs admin', () => { + expect(github.issues.createComment).toHaveBeenCalled(); + const author = payloadData.payload.pull_request.user.login; + const formText = ( + 'server jobs form'.link( + 'https://goo.gl/forms/XIj00RJ2h5L55XzU2') + ); + const newLineFeed = '
'; + const wikiLinkText = 'this guide'.link( + JOBS_AND_FETURES_TESTING_WIKI_LINK); + + expect(github.issues.createComment).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + body: + 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, ' + + 'it adds a new cron job.' + newLineFeed + 'Also @' + author + + ' please add the new test and URL redirects for the new CRON jobs ' + + 'It seems you have added or edited a CRON job, if so please request ' + + 'a testing of this CRON job with this ' + formText + ' Please refer ' + + 'to ' + wikiLinkText + ' for details.' + newLineFeed + 'Thanks!', + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + }); + }); + + it('should assign server jobs admin', () => { + expect(github.issues.addAssignees).toHaveBeenCalled(); + expect(github.issues.addAssignees).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + assignees: ['vojtechjelinek', 'DubeySandeep', 'kevintab95'] + }); + }); + + it('should add datastore label', () => { + expect(github.issues.addLabels).toHaveBeenCalled(); + expect(github.issues.addLabels).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + labels: ['PR: Affects datastore layer'] + }); + }); + }); + + describe('When a new cron job is added in an existing cron job file', () => { + beforeEach(async () => { + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [ + urlJobFileObj, jobTestFile, firstNewJobFileObj + ], + }), + }; + payloadData.payload.pull_request.changed_files = 1; + await robot.receive(payloadData); + }); + + it('should check for cron jobs', () => { + expect(checkCronJobModule.checkForNewCronJob).toHaveBeenCalled(); + }); + + it('should get modified files', () => { + expect(github.pulls.listFiles).toHaveBeenCalled(); + }); + + + it('should ping server jobs admin', () => { + expect(github.issues.createComment).toHaveBeenCalled(); + const author = payloadData.payload.pull_request.user.login; + const formText = ( + 'server jobs form'.link('https://goo.gl/forms/XIj00RJ2h5L55XzU2') + ); + const newLineFeed = '
'; + const wikiLinkText = 'this guide'.link( + JOBS_AND_FETURES_TESTING_WIKI_LINK); + + expect(github.issues.createComment).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + body: + 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, it ' + + 'adds a new cron job.' + newLineFeed + 'Also @' + author + ' It seems' + + ' you have added or edited a CRON job, if so please request a testing' + + ' of this CRON job with this ' + formText + ' Please refer to ' + + wikiLinkText + ' for details.' + newLineFeed + 'Thanks!', + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + }); + }); + + it('should assign server jobs admin', () => { + expect(github.issues.addAssignees).toHaveBeenCalled(); + expect(github.issues.addAssignees).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + assignees: ['vojtechjelinek', 'DubeySandeep', 'kevintab95'] + }); + }); + + it('should add datastore label', () => { + expect(github.issues.addLabels).toHaveBeenCalled(); + expect(github.issues.addLabels).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + labels: ['PR: Affects datastore layer'] + }); + }); + }); + + describe('When no job file is modified in a pull request', () => { + beforeEach(async () => { + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [ + nonJobFile + ], + }), + }; + + payloadData.payload.pull_request.changed_files = 1; + await robot.receive(payloadData); + }); + + it('should check for cron jobs', () => { + expect(checkCronJobModule.checkForNewCronJob).toHaveBeenCalled(); + }); + + it('should not get modified files', () => { + expect(github.pulls.listFiles).toHaveBeenCalled(); + }); + + it('should not ping server job admin', () => { + expect(github.issues.createComment).not.toHaveBeenCalled(); + }); + + it('should not add datastore label', () => { + expect(github.issues.addLabels).not.toHaveBeenCalled(); + }); + }); + + describe('When test and URL redirects are added for a cron job', () => { + beforeEach(async () => { + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [ + jobTestFile, urlJobFileObj + ], + }), + }; + + payloadData.payload.pull_request.changed_files = 1; + await robot.receive(payloadData); + }); + + it('should check for cron jobs', () => { + expect(checkCronJobModule.checkForNewCronJob).toHaveBeenCalled(); + }); + + it('should get modified files', () => { + expect(github.pulls.listFiles).toHaveBeenCalled(); + }); + + it('should ping server job admin', () => { + expect(github.issues.createComment).toHaveBeenCalled(); + const author = payloadData.payload.pull_request.user.login; + const formText = ( + 'server jobs form'.link('https://goo.gl/forms/XIj00RJ2h5L55XzU2') + ); + const newLineFeed = '
'; + const wikiLinkText = 'this guide'.link( + JOBS_AND_FETURES_TESTING_WIKI_LINK); + + expect(github.issues.createComment).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + body: + 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, it ' + + 'adds a new cron job.' + newLineFeed + 'Also @' + author + ' It seems' + + ' you have added or edited a CRON job, if so please request a testing' + + ' of this CRON job with this ' + formText + ' Please refer to ' + + wikiLinkText + ' for details.' + newLineFeed + 'Thanks!', + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + }); + }); + + it('should add datastore label', () => { + expect(github.issues.addLabels).toHaveBeenCalled(); + expect(github.issues.addLabels).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + labels: ['PR: Affects datastore layer'] + }); + }); + }); + + describe('When only tests are added for a cron job', () => { + beforeEach(async () => { + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [ + jobTestFile + ], + }), + }; + + payloadData.payload.pull_request.changed_files = 1; + await robot.receive(payloadData); + }); + + it('should check for cron jobs', () => { + expect(checkCronJobModule.checkForNewCronJob).toHaveBeenCalled(); + }); + + it('should get modified files', () => { + expect(github.pulls.listFiles).toHaveBeenCalled(); + }); + + it('should ping server job admin', () => { + expect(github.issues.createComment).toHaveBeenCalled(); + const author = payloadData.payload.pull_request.user.login; + const formText = ( + 'server jobs form'.link('https://goo.gl/forms/XIj00RJ2h5L55XzU2') + ); + const newLineFeed = '
'; + const wikiLinkText = 'this guide'.link( + JOBS_AND_FETURES_TESTING_WIKI_LINK); + + expect(github.issues.createComment).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + body: + 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, it ' + + 'adds a new cron job.' + newLineFeed + 'Also @' + author + ' It seems' + + ' you have added or edited a CRON job, if so please request a testing' + + ' of this CRON job with this ' + formText + ' Please refer to ' + + wikiLinkText + ' for details.' + newLineFeed + 'Thanks!', + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + }); + }); + + it('should add datastore label', () => { + expect(github.issues.addLabels).toHaveBeenCalled(); + expect(github.issues.addLabels).toHaveBeenCalledWith({ + issue_number: payloadData.payload.pull_request.number, + repo: payloadData.payload.repository.name, + owner: payloadData.payload.repository.owner.login, + labels: ['PR: Affects datastore layer'] + }); + }); + }); + + describe('When pull request has datastore label', () => { + beforeEach(async () => { + payloadData.payload.pull_request.labels = [{ + name: 'PR: Affects datastore layer' + }]; + github.pulls = { + listFiles: jasmine.createSpy('listFiles').and.resolveTo({ + data: [ + nonJobFile, firstNewJobFileObj + ], + }), + }; + + payloadData.payload.pull_request.changed_files = 2; + await robot.receive(payloadData); + }); + + it('should check for cron jobs', () => { + expect(checkCronJobModule.checkForNewCronJob).toHaveBeenCalled(); + }); + + it('should not get modified files', () => { + expect(github.pulls.listFiles).not.toHaveBeenCalled(); + }); + + it('should not ping server job admin', () => { + expect(github.issues.createComment).not.toHaveBeenCalled(); + }); + }); +}); \ No newline at end of file From 52e69df645885cccc0949ec6c5a956ce34bed480 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 14:33:40 +0200 Subject: [PATCH 08/18] Tests --- spec/checkPullRequestJobSpec.js | 77 +++++++++++++-------------------- 1 file changed, 30 insertions(+), 47 deletions(-) diff --git a/spec/checkPullRequestJobSpec.js b/spec/checkPullRequestJobSpec.js index 34057040..45f47812 100644 --- a/spec/checkPullRequestJobSpec.js +++ b/spec/checkPullRequestJobSpec.js @@ -431,7 +431,9 @@ describe('Pull Request Job Spec', () => { app = robot.load(oppiaBot); spyOn(app, 'auth').and.resolveTo(github); - spyOn(checkPullRequestJobModule, 'checkForNewJob').and.callThrough(); + spyOn( + checkPullRequestJobModule, 'checkForModificationsToFiles' + ).and.callThrough(); spyOn(apiForSheetsModule, 'checkClaStatus').and.callFake(() => { }); spyOn( checkCriticalPullRequestModule, 'checkIfPRAffectsDatastoreLayer' @@ -456,7 +458,9 @@ describe('Pull Request Job Spec', () => { }); it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + expect( + checkPullRequestJobModule.checkForModificationsToFiles + ).toHaveBeenCalled(); }); it('should get modified files', () => { @@ -530,7 +534,9 @@ describe('Pull Request Job Spec', () => { }); it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + expect( + checkPullRequestJobModule.checkForModificationsToFiles + ).toHaveBeenCalled(); }); it('should get modified files', () => { @@ -610,7 +616,9 @@ describe('Pull Request Job Spec', () => { }); it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + expect( + checkPullRequestJobModule.checkForModificationsToFiles + ).toHaveBeenCalled(); }); it('should get modified files', () => { @@ -681,7 +689,9 @@ describe('Pull Request Job Spec', () => { }); it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + expect( + checkPullRequestJobModule.checkForModificationsToFiles + ).toHaveBeenCalled(); }); it('should get modified files', () => { @@ -761,7 +771,9 @@ describe('Pull Request Job Spec', () => { }); it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + expect( + checkPullRequestJobModule.checkForModificationsToFiles + ).toHaveBeenCalled(); }); it('should get modified files', () => { @@ -795,7 +807,9 @@ describe('Pull Request Job Spec', () => { }); it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + expect( + checkPullRequestJobModule.checkForModificationsToFiles + ).toHaveBeenCalled(); }); it('should not get modified files', () => { @@ -822,7 +836,9 @@ describe('Pull Request Job Spec', () => { }); it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + expect( + checkPullRequestJobModule.checkForModificationsToFiles + ).toHaveBeenCalled(); }); it('should get modified files', () => { @@ -849,7 +865,9 @@ describe('Pull Request Job Spec', () => { }); it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + expect( + checkPullRequestJobModule.checkForModificationsToFiles + ).toHaveBeenCalled(); }); it('should get modified files', () => { @@ -879,7 +897,9 @@ describe('Pull Request Job Spec', () => { }); it('should check for jobs', () => { - expect(checkPullRequestJobModule.checkForNewJob).toHaveBeenCalled(); + expect( + checkPullRequestJobModule.checkForModificationsToFiles + ).toHaveBeenCalled(); }); it('should not get modified files', () => { @@ -890,41 +910,4 @@ describe('Pull Request Job Spec', () => { expect(github.issues.createComment).not.toHaveBeenCalled(); }); }); - - describe('Returns appropriate job name', () => { - it('should return the correct job created in the file', () => { - let jobs = checkPullRequestJobModule.getNewJobsFromFile( - firstNewJobFileObj - ); - expect(jobs.length).toBe(1); - expect(jobs[0]).toBe('FirstTestJob'); - - jobs = checkPullRequestJobModule.getNewJobsFromFile(secondNewJobFileObj); - expect(jobs.length).toBe(1); - expect(jobs[0]).toBe('SecondTestJob'); - - jobs = checkPullRequestJobModule.getNewJobsFromFile( - modifiedExistingJobFileObj - ); - expect(jobs.length).toBe(1); - expect(jobs[0]).toBe('OppiabotContributionsJob'); - - jobs = checkPullRequestJobModule.getNewJobsFromFile(fileWithMultipleJobs); - expect(jobs.length).toBe(2); - expect(jobs[0]).toBe('TestJob'); - expect(jobs[1]).toBe('AnotherTestJob'); - - jobs = checkPullRequestJobModule.getNewJobsFromFile(jobTestFile); - expect(jobs.length).toBe(0); - - jobs = checkPullRequestJobModule.getNewJobsFromFile( - modifiedExistingJobFileObjWithJobClassInPatch); - expect(jobs.length).toBe(0); - - jobs = checkPullRequestJobModule.getNewJobsFromFile( - jobFileObjWithJobClassInPatchAndNewJob); - expect(jobs.length).toBe(1); - expect(jobs[0]).toBe('AnotherTestJob'); - }); - }); }); \ No newline at end of file From ccfc1af61c1f16ad626438fb42594d5bd0a6d165 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 14:51:48 +0200 Subject: [PATCH 09/18] Tests --- lib/checkPullRequestJob.js | 11 ++++--- spec/checkPullRequestJobSpec.js | 58 +++++---------------------------- 2 files changed, 16 insertions(+), 53 deletions(-) diff --git a/lib/checkPullRequestJob.js b/lib/checkPullRequestJob.js index 15ec5e6d..053d0674 100644 --- a/lib/checkPullRequestJob.js +++ b/lib/checkPullRequestJob.js @@ -28,10 +28,13 @@ const PLATFORM_DIR_PREFIX = 'core/platform/'; /** * @param {string} filename */ -const isInDirsThatShouldBeTested = (filename) => { +const fileChangesShouldBeVerified = (filename) => { return ( - filename.startsWith(JOBS_DIR_PREFIX) || - filename.startsWith(PLATFORM_DIR_PREFIX) + ( + filename.startsWith(JOBS_DIR_PREFIX) || + filename.startsWith(PLATFORM_DIR_PREFIX) + ) && + !filename.endsWith('test.py') ); }; @@ -75,7 +78,7 @@ const checkForModificationsToFiles = async (context) => { // Get new jobs that were created in the PR. const modifiedFilesToBeTested = changedFiles.filter((file) => { - return isInDirsThatShouldBeTested(file.filename); + return fileChangesShouldBeVerified(file.filename); }); if (modifiedFilesToBeTested.length > 0) { diff --git a/spec/checkPullRequestJobSpec.js b/spec/checkPullRequestJobSpec.js index 45f47812..8f21eac6 100644 --- a/spec/checkPullRequestJobSpec.js +++ b/spec/checkPullRequestJobSpec.js @@ -484,10 +484,10 @@ describe('Pull Request Job Spec', () => { issue_number: payloadData.payload.pull_request.number, body: 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, ' + - 'it adds a new job. The name of the job is ' + jobNameLink + '.' + + 'it modifies files in jobs or platform folders.' + newLineFeed + 'Also @' + author + ', please make sure to fill in the ' + formText + - ' for the new job to be tested on the backup server. ' + + ' for the new job or feature to be tested on the backup server. ' + 'This PR can be merged only after the test run is successful. ' + 'Please refer to ' + wikiLinkText + ' for details.' + newLineFeed + 'Thanks!', @@ -641,10 +641,10 @@ describe('Pull Request Job Spec', () => { issue_number: payloadData.payload.pull_request.number, body: 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this ' + - 'PR, it adds a new job. The name of the job is ' + jobNameLink + + 'PR, it modifies files in jobs or platform folders.' + '.' + newLineFeed + 'Also @' + author + ', please make sure to fill in the ' + formText + - ' for the new job to be tested on the backup server. ' + + ' for the new job or feature to be tested on the backup server. ' + 'This PR can be merged only after the test run is successful. ' + 'Please refer to ' + wikiLinkText + ' for details.' + newLineFeed + 'Thanks!', @@ -720,11 +720,11 @@ describe('Pull Request Job Spec', () => { issue_number: payloadData.payload.pull_request.number, body: 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, ' + - 'it adds a new job. The name of the job is ' + jobNameLink + - '.' + newLineFeed + 'Also @' + author + ', please make sure to ' + - 'fill in the ' + formText + ' for the new job to be tested on the ' + - 'backup server. ' + 'This PR can be merged only after the test ' + - 'run is successful. ' + 'Please refer to ' + wikiLinkText + + 'it modifies files in jobs or platform folders.' + newLineFeed + + 'Also @' + author + ', please make sure to ' + + 'fill in the ' + formText + ' for the new job or feature to be ' + + 'tested on the backup server. This PR can be merged only after the test ' + + 'run is successful. Please refer to ' + wikiLinkText + ' for details.' + newLineFeed + 'Thanks!', repo: payloadData.payload.repository.name, owner: payloadData.payload.repository.owner.login, @@ -752,46 +752,6 @@ describe('Pull Request Job Spec', () => { }); }); - describe('When an existing job file is modified with no new job', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [ - { - ...firstNewJobFileObj, - status: 'modified', - patch: '\n+# No job files present in the changes', - }, - ], - }), - }; - - payloadData.payload.pull_request.changed_files = 1; - await robot.receive(payloadData); - }); - - it('should check for jobs', () => { - expect( - checkPullRequestJobModule.checkForModificationsToFiles - ).toHaveBeenCalled(); - }); - - it('should get modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - - it('should not ping server jobs admin', () => { - expect(github.issues.createComment).not.toHaveBeenCalled(); - }); - it('should not add datastore label', () => { - expect(github.issues.addLabels).not.toHaveBeenCalled(); - }); - it('should not assign server jobs admin', () => { - expect(github.issues.addAssignees).not.toHaveBeenCalled(); - }); - }); - describe('When no job file is modified in a pull request', () => { beforeEach(async () => { github.pulls = { From be79d2c871bd944908579d7c7dba47cddb574307 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 14:58:39 +0200 Subject: [PATCH 10/18] Tests --- spec/checkNewCronJobSpec.js | 10 +++++----- spec/checkPullRequestJobSpec.js | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/checkNewCronJobSpec.js b/spec/checkNewCronJobSpec.js index 998920c0..4bc32e63 100644 --- a/spec/checkNewCronJobSpec.js +++ b/spec/checkNewCronJobSpec.js @@ -28,7 +28,7 @@ const checkPullRequestTemplateModule = require('../lib/checkPullRequestTemplate'); const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); const scheduler = require('../lib/scheduler'); -const { JOBS_AND_FETURES_TESTING_WIKI_LINK } = require('../lib/utils'); +const { JOBS_AND_FEATURES_TESTING_WIKI_LINK } = require('../lib/utils'); let payloadData = JSON.parse( JSON.stringify(require('../fixtures/pullRequestPayload.json')) ); @@ -242,7 +242,7 @@ describe('Cron Job Spec', () => { ); const newLineFeed = '
'; const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); + JOBS_AND_FEATURES_TESTING_WIKI_LINK); expect(github.issues.createComment).toHaveBeenCalledWith({ issue_number: payloadData.payload.pull_request.number, @@ -309,7 +309,7 @@ describe('Cron Job Spec', () => { ); const newLineFeed = '
'; const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); + JOBS_AND_FEATURES_TESTING_WIKI_LINK); expect(github.issues.createComment).toHaveBeenCalledWith({ issue_number: payloadData.payload.pull_request.number, @@ -406,7 +406,7 @@ describe('Cron Job Spec', () => { ); const newLineFeed = '
'; const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); + JOBS_AND_FEATURES_TESTING_WIKI_LINK); expect(github.issues.createComment).toHaveBeenCalledWith({ issue_number: payloadData.payload.pull_request.number, @@ -462,7 +462,7 @@ describe('Cron Job Spec', () => { ); const newLineFeed = '
'; const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); + JOBS_AND_FEATURES_TESTING_WIKI_LINK); expect(github.issues.createComment).toHaveBeenCalledWith({ issue_number: payloadData.payload.pull_request.number, diff --git a/spec/checkPullRequestJobSpec.js b/spec/checkPullRequestJobSpec.js index 8f21eac6..d3937341 100644 --- a/spec/checkPullRequestJobSpec.js +++ b/spec/checkPullRequestJobSpec.js @@ -26,7 +26,7 @@ const checkPullRequestTemplateModule = const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); const scheduler = require('../lib/scheduler'); const checkCronJobModule = require('../lib/checkNewCronJobs'); -const { JOBS_AND_FETURES_TESTING_WIKI_LINK } = require('../lib/utils'); +const { JOBS_AND_FEATURES_TESTING_WIKI_LINK } = require('../lib/utils'); let payloadData = JSON.parse( JSON.stringify(require('../fixtures/pullRequestPayload.json')) @@ -475,7 +475,7 @@ describe('Pull Request Job Spec', () => { ); const newLineFeed = '
'; const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); + JOBS_AND_FEATURES_TESTING_WIKI_LINK); const jobNameLink = ( 'FirstTestJob'.link(firstNewJobFileObj.blob_url) ); @@ -551,7 +551,7 @@ describe('Pull Request Job Spec', () => { ); const newLineFeed = '
'; const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); + JOBS_AND_FEATURES_TESTING_WIKI_LINK); const jobRegistryLink = ( 'job registry'.link( 'https://github.com/oppia/oppia/blob/develop/core/jobs_registry.py') @@ -633,7 +633,7 @@ describe('Pull Request Job Spec', () => { ); const newLineFeed = '
'; const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); + JOBS_AND_FEATURES_TESTING_WIKI_LINK); const jobNameLink = ( 'FirstTestJob'.link(firstNewJobFileObj.blob_url) ); @@ -707,7 +707,7 @@ describe('Pull Request Job Spec', () => { ); const newLineFeed = '
'; const wikiLinkText = 'this guide'.link( - JOBS_AND_FETURES_TESTING_WIKI_LINK); + JOBS_AND_FEATURES_TESTING_WIKI_LINK); const jobRegistryLink = ( 'job registry'.link( 'https://github.com/oppia/oppia/blob/develop/core/jobs_registry.py') From 9c75861c9be7d99b2dbf413ccfdace98eb28a15b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 15:10:25 +0200 Subject: [PATCH 11/18] Tests and remove codeowner check --- constants.js | 4 - index.js | 4 - lib/checkForNewCodeowner.js | 163 ------------ spec/checkForNewCodeownerSpec.js | 422 ------------------------------- spec/checkPullRequestJobSpec.js | 8 +- userWhitelist.json | 1 - 6 files changed, 3 insertions(+), 599 deletions(-) delete mode 100644 lib/checkForNewCodeowner.js delete mode 100644 spec/checkForNewCodeownerSpec.js diff --git a/constants.js b/constants.js index f8060ef5..1ef2907a 100644 --- a/constants.js +++ b/constants.js @@ -52,7 +52,6 @@ const issuesLabelCheck = 'issues-labeled-check'; const issuesAssignedCheck = 'issues-assigned-check'; const forcePushCheck = 'force-push-check'; const pullRequestReviewCheck = 'pr-review-check'; -const codeOwnerCheck = 'code-owner-check'; const ciFailureCheck = 'ci-failure-check'; const updateWithDevelopCheck = 'update-with-develop-check'; const respondToReviewCheck = 'respond-to-review-check'; @@ -79,7 +78,6 @@ const checksWhitelist = { [openEvent]: [ assigneeCheck, claCheck, - codeOwnerCheck, branchCheck, jobCheck, cronJobCheck, @@ -102,7 +100,6 @@ const checksWhitelist = { jobCheck, cronJobCheck, modelCheck, - codeOwnerCheck, ], [closeEvent]: [allMergeConflictCheck, updateWithDevelopCheck], [editEvent]: [wipCheck], @@ -169,7 +166,6 @@ module.exports.hotfixLabelCheck = hotfixLabelCheck; module.exports.prTemplateCheck = prTemplateCheck; module.exports.forcePushCheck = forcePushCheck; module.exports.pullRequestReviewCheck = pullRequestReviewCheck; -module.exports.codeOwnerCheck = codeOwnerCheck; module.exports.ciFailureCheck = ciFailureCheck; module.exports.updateWithDevelopCheck = updateWithDevelopCheck; module.exports.respondToReviewCheck = respondToReviewCheck; diff --git a/index.js b/index.js index d689a3ff..cc83d7c5 100644 --- a/index.js +++ b/index.js @@ -32,7 +32,6 @@ const checkCriticalPullRequestModule = require( ); const checkBranchPushModule = require('./lib/checkBranchPush'); const checkPullRequestReviewModule = require('./lib/checkPullRequestReview'); -const newCodeOwnerModule = require('./lib/checkForNewCodeowner'); const ciCheckModule = require('./lib/ciChecks'); const periodicCheckModule = require('./lib/periodicChecks'); @@ -133,9 +132,6 @@ const runChecks = async (context, checkEvent) => { checkPullRequestReviewModule.handlePullRequestReview(context) ); break; - case constants.codeOwnerCheck: - callable.push(newCodeOwnerModule.checkForNewCodeowner(context)); - break; case constants.ciFailureCheck: callable.push(ciCheckModule.handleFailure(context)); break; diff --git a/lib/checkForNewCodeowner.js b/lib/checkForNewCodeowner.js deleted file mode 100644 index 02995af0..00000000 --- a/lib/checkForNewCodeowner.js +++ /dev/null @@ -1,163 +0,0 @@ -// Copyright 2020 The Oppia Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS-IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -/** - * @fileoverview File to handle checks when a new code owner gets added. - */ - -const utilsModule = require('./utils'); -const userWhitelist = require('../userWhitelist.json'); -const CODE_OWNER_FILE_NAME = '.github/CODEOWNERS'; -const newAddition = '+'; -const newCommentAddition = '+#'; -const newLine = '\n'; -const usernameDefiner = '@'; - -/** - * This function returns true if new added line starts with '+' - * and not as '+#' which is comment line and false if otherwise. - * - * @param {string} change - */ -const checkNewLineAddition = (change) => { - return ( - change.startsWith(newAddition) && !change.startsWith(newCommentAddition) - ); -}; - -/** - * This function returns all the new codeowners added to the codeowners file. - * @param {import('probot').Octokit.PullsListFilesResponseItem} file - * @returns {string[]} - */ -const getNewCodeOwners = (file) => { - const changesArray = file.patch.split(newLine); - const newAdditions = changesArray.filter((change) => { - return checkNewLineAddition(change); - }); - - /** - * @type {string[]} addedCodeOwners - */ - let addedCodeOwners = []; - newAdditions.forEach((addition) => { - const usernames = addition.split(usernameDefiner); - // The first item of the array will be the path to the file, and should - // be removed to get only the usernames. - usernames.shift(); - addedCodeOwners.push(...usernames); - }); - // Trim and add @ to the front of the username. - addedCodeOwners = addedCodeOwners.map( - (username) => usernameDefiner + username.trim() - ); - - // Remove all duplicates and return the array. - return Array.from(new Set(addedCodeOwners)); -}; - -/** - * This function returns all the files added/modified to the codeowner file. - * @param {import('probot').Octokit.PullsListFilesResponseItem} file - * @returns {string[]} - */ -const getNewCodeOwnerFiles = (file) => { - const changesArray = file.patch.split(newLine); - const newAdditions = changesArray.filter((change) => { - return checkNewLineAddition(change); - }); - - const addedFiles = newAdditions.map((addition) => { - const additionArray = addition.split(usernameDefiner); - // The first item of the array will be the path to the file, and should - // be returned. - const filePath = additionArray.shift().trim(); - // Remove '+' from the file path. - return filePath.substring(1); - }); - - return addedFiles; -}; - -/** - * This function checks if a new codeowner has been added to the codeowner - * file and responds as required. - * - * @param {import('probot').Context} context - */ -const checkForNewCodeowner = async (context) => { - /** - * @type {import('probot').Octokit.PullsGetResponse} pullRequest - */ - const pullRequest = context.payload.pull_request; - - const changedFiles = await utilsModule.getAllChangedFiles(context); - - const changedCodeOwnerFile = changedFiles.find( - (file) => file.filename === CODE_OWNER_FILE_NAME - ); - - if (changedCodeOwnerFile) { - const mainCodeOwnerFile = await utilsModule.getMainCodeOwnerfile(); - - const newlyAddedCodeOwners = getNewCodeOwners(changedCodeOwnerFile); - - // Remove all exisiting codeowners. - const newCodeOwners = newlyAddedCodeOwners.filter( - (user) => !mainCodeOwnerFile.includes(user) - ); - - if (newCodeOwners.length > 0) { - let codeOwnerString = ''; - if (newCodeOwners.length > 1) { - codeOwnerString = - 'the following new code owners ' + newCodeOwners.join(', '); - } else { - codeOwnerString = 'a new code owner, ' + newCodeOwners[0]; - } - - const filePaths = getNewCodeOwnerFiles(changedCodeOwnerFile); - let codeOwnerFileString = ''; - if (filePaths.length > 1) { - codeOwnerFileString = ' to the following files ' + filePaths.join(', '); - } else { - codeOwnerFileString = ' to ' + filePaths[0]; - } - - let commentBody = ''; - // Ping the code owner of CODEOWNERS file since we just want to flag - // that a new code-owner is added and bring it to his attention. - commentBody = - 'Hi @' + - userWhitelist.codeOwnerFileReviewer + - ', this PR adds ' + - codeOwnerString + - codeOwnerFileString + - '. Please make sure the changes are ' + - 'verified by the previous codeowner(s) of the file. Thanks!'; - - context.github.issues.createComment( - context.repo({ - issue_number: pullRequest.number, - body: commentBody, - }) - ); - } - } -}; - -module.exports = { - checkForNewCodeowner, - getNewCodeOwners -}; diff --git a/spec/checkForNewCodeownerSpec.js b/spec/checkForNewCodeownerSpec.js deleted file mode 100644 index 5e87c609..00000000 --- a/spec/checkForNewCodeownerSpec.js +++ /dev/null @@ -1,422 +0,0 @@ -// Copyright 2020 The Oppia Authors. All Rights Reserved. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS-IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -/** - * @fileoverview Spec for new code owner checks. - */ - -require('dotenv').config(); -const { createProbot } = require('probot'); -// The plugin refers to the actual app in index.js. -const oppiaBot = require('../index'); -const checkPullRequestJobModule = require('../lib/checkPullRequestJob'); -const checkCronJobModule = require('../lib/checkNewCronJobs'); -const checkCriticalPullRequestModule = - require('../lib/checkCriticalPullRequest'); -const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); -const mergeConflictModule = require('../lib/checkMergeConflicts'); -const utils = require('../lib/utils'); -const scheduler = require('../lib/scheduler'); -let payloadData = JSON.parse( - JSON.stringify(require('../fixtures/pullRequestPayload.json')) -); - -describe('Check for new code owner', () => { - /** - * @type {import('probot').Probot} robot - */ - let robot; - - /** - * @type {import('probot').Octokit} github - */ - let github; - - /** - * @type {import('probot').Application} app - */ - let app; - - const mainCodeOwnerFile = `# Speed Improvement team. - /app_dev.yaml @vojtechjelinek - /core/templates/google-analytics.initializer.ts @jameesjohn @vojtechjelinek - /core/templates/base-components/ @jameesjohn @vojtechjelinek - /core/templates/pages/Base.ts @jameesjohn @vojtechjelinek - /core/templates/pages/oppia_footer_directive.html - @jameesjohn @vojtechjelinek - /core/templates/pages/OppiaFooterDirective.ts @jameesjohn @vojtechjelinek - /core/templates/pages/footer_js_libs.html @jameesjohn @vojtechjelinek - /core/templates/pages/header_css_libs.html @jameesjohn @vojtechjelinek - /core/templates/services/csrf-token.service*.ts @jameesjohn @vojtechjelinek - /core/templates/third-party-imports/ @nishantwrp @vojtechjelinek - /jinja_utils*.py @jameesjohn @vojtechjelinek - /.lighthouserc.json @jimbyo @jameesjohn @vojtechjelinek - /puppeteer-login-script.js @jimbyo - /scripts/run_lighthouse_tests.py @jameesjohn @vojtechjelinek - /webpack.common.config.ts @jameesjohn @vojtechjelinek - /webpack.common.macros.ts @jameesjohn @vojtechjelinek - /webpack.dev.config.ts @jameesjohn @vojtechjelinek - /webpack.prod.config.ts @jameesjohn @vojtechjelinek - /webpack.terser.config.ts @nishantwrp @vojtechjelinek`; - - const nonCodeOwnerFile = { - sha: 'd144f32b9812373d5f1bc9f94d9af795f09023ff', - filename: 'core/domain/exp_fetchers.py', - status: 'added', - additions: 1, - deletions: 0, - changes: 1, - blob_url: - 'https://github.com/oppia/oppia/blob/67fb4a973b318882af3b5a894130' + - 'e110d7e9833c/core/domain/exp_fetchers.py', - raw_url: - 'https://github.com/oppia/oppia/raw/67fb4a973b318882af3b5a894130e' + - '110d7e9833c/core/domain/exp_fetchers.py', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/core/domain/exp' + - '_fetchers.py?ref=67fb4a973b318882af3b5a894130e110d7e9833c', - patch: - '@@ -0,0 +1 @@\n+# def _migrate_states_schema(versioned_exploration' + - '_states, exploration_id):', - }; - - const codeOwnerFileWithNewUser = { - sha: '6911619e95dfcb11eb2204fba88987e5abf02352', - filename: '.github/CODEOWNERS', - status: 'modified', - additions: 2, - deletions: 0, - changes: 2, - blob_url: - 'https://github.com/oppia/oppia/blob/c876111ec9c743179483d7ca75e348' + - 'b8680b13ec/.github/CODEOWNERS', - raw_url: - 'https://github.com/oppia/oppia/raw/c876111ec9c743179483d7ca75e348b8' + - '680b13ec/.github/CODEOWNERS', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/.github/CODEOWNERS' + - '?ref=c876111ec9c743179483d7ca75e348b8680b13ec', - patch: - '@@ -523,7 +523,9 @@\n' + - ' /core/templates/pages/pending-account-deletion-page/ ' + - '@jameesjohn @vojtechjelinek\n' + - ' /core/templates/pages/preferences-page/ @jameesjohn @vojtechjelinek\n' + - '+/core/templates/pages/signup-page/ @testuser @vojtechjelinek\n' + - ' /core/templates/pages/splash-page/ @jameesjohn @vojtechjelinek\n' + - ' /core/templates/pages/teach-page/ @jameesjohn @vojtechjelinek\n' + - ' /core/templates/pages/thanks-page/ @jameesjohn @vojtechjelinek\n' + - ' ', - }; - - const codeOwnerFileWithMultipleNewUsers = { - sha: '6911619e95dfcb11eb2204fba88987e5abf02352', - filename: '.github/CODEOWNERS', - status: 'modified', - additions: 2, - deletions: 0, - changes: 2, - blob_url: - 'https://github.com/oppia/oppia/blob/c876111ec9c743179483d7ca75e348b' + - '8680b13ec/.github/CODEOWNERS', - raw_url: - 'https://github.com/oppia/oppia/raw/c876111ec9c743179483d7ca75e348b8' + - '680b13ec/.github/CODEOWNERS', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/.github/CODEOWNERS' + - '?ref=c876111ec9c743179483d7ca75e348b8680b13ec', - patch: - '@@ -523,7 +523,9 @@\n' + - ' /core/templates/pages/pending-account-deletion-page/ ' + - '@jameesjohn @vojtechjelinek\n' + - ' /core/templates/pages/preferences-page/ @jameesjohn @vojtechjelinek\n' + - ' /core/templates/pages/signup-page/ @jameesjohn @vojtechjelinek\n' + - '+/core/templates/pages/signup-page/ @testuser @vojtechjelinek\n' + - ' /core/templates/pages/splash-page/ @jameesjohn @vojtechjelinek\n' + - '+/core/templates/pages/signdown-page/ @testuser2 @jameesjohn\n' + - ' /core/templates/pages/teach-page/ @jameesjohn @vojtechjelinek\n' + - ' /core/templates/pages/thanks-page/ @jameesjohn @vojtechjelinek\n' + - ' ', - }; - - const codeOwnerFileWithNewExistingUser = { - sha: '6911619e95dfcb11eb2204fba88987e5abf02352', - filename: '.github/CODEOWNERS', - status: 'modified', - additions: 2, - deletions: 0, - changes: 2, - blob_url: - 'https://github.com/oppia/oppia/blob/c876111ec9c743179483d7ca75e348b86' + - '80b13ec/.github/CODEOWNERS', - raw_url: - 'https://github.com/oppia/oppia/raw/c876111ec9c743179483d7ca75e348b868' + - '0b13ec/.github/CODEOWNERS', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/.github/CODEOWNERS' + - '?ref=c876111ec9c743179483d7ca75e348b8680b13ec', - patch: - '@@ -523,7 +523,9 @@\n' + - ' /core/templates/pages/pending-account-deletion-page/ ' + - '@jameesjohn @vojtechjelinek\n' + - ' /core/templates/pages/preferences-page/ @jameesjohn @vojtechjelinek\n' + - ' /core/templates/pages/signup-page/ @jameesjohn @vojtechjelinek\n' + - '+/core/templates/pages/signup-page/ @jameesjohn @vojtechjelinek\n' + - ' /core/templates/pages/splash-page/ @jameesjohn @vojtechjelinek\n' + - '+/core/templates/pages/signup-page/ @vojtechjelinek @jameesjohn\n' + - ' /core/templates/pages/teach-page/ @jameesjohn @vojtechjelinek\n' + - ' /core/templates/pages/thanks-page/ @jameesjohn @vojtechjelinek\n' + - ' ', - }; - beforeAll(() => { - payloadData.payload.action = 'synchronize'; - }); - - beforeEach(() => { - spyOn(scheduler, 'createScheduler').and.callFake(() => { }); - - github = { - issues: { - createComment: jasmine.createSpy('createComment').and.returnValue({}), - }, - }; - - robot = createProbot({ - id: 1, - cert: 'test', - githubToken: 'test', - }); - - app = robot.load(oppiaBot); - spyOn(app, 'auth').and.resolveTo(github); - spyOn( - checkPullRequestJobModule, 'checkForModificationsToFiles' - ).and.callFake(() => { }); - spyOn(checkCronJobModule, 'checkForNewCronJob').and.callFake(() => { }); - spyOn( - checkCriticalPullRequestModule, - 'checkIfPRAffectsDatastoreLayer' - ).and.callFake(() => { }); - spyOn(mergeConflictModule, 'checkMergeConflictsInPullRequest') - .and.callFake(() => { }); - spyOn(newCodeOwnerModule, 'checkForNewCodeowner').and.callThrough(); - spyOn(utils, 'getMainCodeOwnerfile').and.resolveTo(mainCodeOwnerFile); - }); - - describe('When a new code owner gets added', () => { - beforeEach(() => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [nonCodeOwnerFile, codeOwnerFileWithNewUser], - }), - }; - payloadData.payload.pull_request.changed_files = 2; - }); - - describe('and there is no changelog label', () => { - beforeEach(async () => { - const reviewer = { - login: 'reviewer' - }; - payloadData.payload.pull_request.requested_reviewers.push(reviewer); - await robot.receive(payloadData); - }); - - it('should check for new code owner', () => { - expect(newCodeOwnerModule.checkForNewCodeowner).toHaveBeenCalled(); - }); - - it('should get all modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - it('should get codeowner file from develop', () => { - expect(utils.getMainCodeOwnerfile).toHaveBeenCalled(); - }); - - it('should ping reviewer', () => { - expect(github.issues.createComment).toHaveBeenCalledWith({ - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - issue_number: payloadData.payload.pull_request.number, - body: - 'Hi @DubeySandeep, this PR adds a new code owner, @testuser to ' + - '/core/templates/pages/signup-page/. Please make sure the changes' + - ' are verified by the previous codeowner(s) of the file. Thanks!' - }); - }); - - afterEach(() => { - payloadData.payload.pull_request.requested_reviewers.pop(); - }); - }); - }); - - describe('When multiple new code owners get added', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [nonCodeOwnerFile, codeOwnerFileWithMultipleNewUsers], - }), - }; - payloadData.payload.pull_request.changed_files = 2; - await robot.receive(payloadData); - }); - it('should check for new code owner', () => { - expect(newCodeOwnerModule.checkForNewCodeowner).toHaveBeenCalled(); - }); - - it('should get all modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - it('should get codeowner file from develop', () => { - expect(utils.getMainCodeOwnerfile).toHaveBeenCalled(); - }); - - it('should ping project owner', () => { - expect(github.issues.createComment).toHaveBeenCalled(); - expect(github.issues.createComment).toHaveBeenCalledWith({ - repo: payloadData.payload.repository.name, - owner: payloadData.payload.repository.owner.login, - issue_number: payloadData.payload.pull_request.number, - body: - 'Hi @DubeySandeep, this PR adds the following new code owners ' + - '@testuser, @testuser2 to the following files ' + - '/core/templates/pages/signup-page/, ' + - '/core/templates/pages/signdown-page/' + - '. Please make sure the changes are ' + - 'verified by the previous codeowner(s) of the file. Thanks!' - }); - }); - - afterEach(() => { - payloadData.payload.pull_request.labels.pop(); - }); - }); - - describe('When an exisiting code owner gets added to another file', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [nonCodeOwnerFile, codeOwnerFileWithNewExistingUser], - }), - }; - payloadData.payload.pull_request.changed_files = 2; - await robot.receive(payloadData); - }); - it('should check for new code owner', () => { - expect(newCodeOwnerModule.checkForNewCodeowner).toHaveBeenCalled(); - }); - - it('should get all modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - it('should get codeowner file from develop', () => { - expect(utils.getMainCodeOwnerfile).toHaveBeenCalled(); - }); - - it('should not ping project owner', () => { - expect(github.issues.createComment).not.toHaveBeenCalled(); - }); - - afterEach(() => { - payloadData.payload.pull_request.labels.pop(); - }); - }); - - describe('When codeowner file is not modified', () => { - beforeEach(async () => { - github.pulls = { - listFiles: jasmine.createSpy('listFiles').and.resolveTo({ - data: [nonCodeOwnerFile], - }), - }; - payloadData.payload.pull_request.changed_files = 1; - await robot.receive(payloadData); - }); - it('should check for new code owner', () => { - expect(newCodeOwnerModule.checkForNewCodeowner).toHaveBeenCalled(); - }); - - it('should get all modified files', () => { - expect(github.pulls.listFiles).toHaveBeenCalled(); - }); - - it('should not get codeowner file from develop', () => { - expect(utils.getMainCodeOwnerfile).not.toHaveBeenCalled(); - }); - - it('should not ping project owner', () => { - expect(github.issues.createComment).not.toHaveBeenCalled(); - }); - - afterEach(() => { - payloadData.payload.pull_request.labels.pop(); - }); - }); - - it('Should get new codeowner from files', () => { - spyOn(newCodeOwnerModule, 'getNewCodeOwners').and.callThrough(); - - // Payload from https://github.com/oppia/oppia/pull/10534. - const fileWithComments = { - sha: '6911619e95dfcb11eb2204fba88987e5abf02352', - filename: '.github/CODEOWNERS', - status: 'modified', - additions: 82, - deletions: 58, - changes: 140, - blob_url: - 'https://github.com/oppia/oppia/blob/c876111ec9c743179483d7ca75e348b' + - '8680b13ec/.github/CODEOWNERS', - raw_url: - 'https://github.com/oppia/oppia/raw/c876111ec9c743179483d7ca75e348b8' + - '680b13ec/.github/CODEOWNERS', - contents_url: - 'https://api.github.com/repos/oppia/oppia/contents/.github/CODEOWNERS' + - '?ref=c876111ec9c743179483d7ca75e348b8680b13ec', - patch: - '-/extensions/ @vojtechjelinek\r\n+# TODO(#10538): Revert ownership ' + - 'to @vojtechjelinek after 2020-09-06.\r\n+/extensions/ @seanlip\r\n/' + - 'core/templates/services/UpgradedServices.ts @bansalnitish ' + - '@srijanreddy98\r\n/typings/ @ankita240796\r\n/tsconfig.json ' + - '@ankita240796\r\n-/tsconfig-strict.json @nishantwrp @vojtechjelinek' + - '\r\n+# TODO(#10538): Add @vojtechjelinek as an owner after 2020-09-' + - '06.\r\n+/tsconfig-strict.json @nishantwrp\r\n-/core/controllers/' + - 'resources*.py @vojtechjelinek\r\n+# TODO(#10538): Revert ownership ' + - 'to @vojtechjelinek after 2020-09-06.\r\n-/core/controllers/' + - 'resources*.py @seanlip' - }; - - let codeowners = newCodeOwnerModule.getNewCodeOwners(fileWithComments); - expect(codeowners.length).toBe(2); - expect(codeowners).toEqual(['@seanlip', '@nishantwrp']); - - codeowners = newCodeOwnerModule.getNewCodeOwners(nonCodeOwnerFile); - expect(codeowners.length).toBe(0); - - codeowners = newCodeOwnerModule.getNewCodeOwners(codeOwnerFileWithNewUser); - expect(codeowners.length).toBe(2); - expect(codeowners).toEqual(['@testuser', '@vojtechjelinek']); - - codeowners = newCodeOwnerModule.getNewCodeOwners( - codeOwnerFileWithMultipleNewUsers - ); - expect(codeowners.length).toBe(4); - expect(codeowners).toEqual([ - '@testuser', '@vojtechjelinek', '@testuser2', '@jameesjohn' - ]); - }); -}); diff --git a/spec/checkPullRequestJobSpec.js b/spec/checkPullRequestJobSpec.js index d3937341..51253e9b 100644 --- a/spec/checkPullRequestJobSpec.js +++ b/spec/checkPullRequestJobSpec.js @@ -484,8 +484,7 @@ describe('Pull Request Job Spec', () => { issue_number: payloadData.payload.pull_request.number, body: 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, ' + - 'it modifies files in jobs or platform folders.' + - newLineFeed + + 'it modifies files in jobs or platform folders.' + newLineFeed + 'Also @' + author + ', please make sure to fill in the ' + formText + ' for the new job or feature to be tested on the backup server. ' + 'This PR can be merged only after the test run is successful. ' + @@ -641,9 +640,8 @@ describe('Pull Request Job Spec', () => { issue_number: payloadData.payload.pull_request.number, body: 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this ' + - 'PR, it modifies files in jobs or platform folders.' + - '.' + newLineFeed + 'Also @' + author + - ', please make sure to fill in the ' + formText + + 'PR, it modifies files in jobs or platform folders.' + newLineFeed + + 'Also @' + author + ', please make sure to fill in the ' + formText + ' for the new job or feature to be tested on the backup server. ' + 'This PR can be merged only after the test run is successful. ' + 'Please refer to ' + wikiLinkText + ' for details.' + diff --git a/userWhitelist.json b/userWhitelist.json index f265951a..5ad67a45 100644 --- a/userWhitelist.json +++ b/userWhitelist.json @@ -27,7 +27,6 @@ "aks681", "srijanreddy98" ], - "codeOwnerFileReviewer":"DubeySandeep", "teamLeads": { "onboardingTeam": "DubeySandeep", "releaseTeam": "vojtechjelinek", From c3a5cec46d61c942af3f5fc45241ac03f18d27ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 15:15:45 +0200 Subject: [PATCH 12/18] Tests --- spec/apiForSheetsSpec.js | 2 -- spec/checkCriticalPullRequestSpec.js | 2 -- spec/checkMergeConflictSpec.js | 2 -- spec/checkNewCronJobSpec.js | 2 -- spec/checkPullRequestBranchSpec.js | 2 -- spec/checkPullRequestJobSpec.js | 2 -- spec/checkPullRequestTemplateSpec.js | 2 -- 7 files changed, 14 deletions(-) diff --git a/spec/apiForSheetsSpec.js b/spec/apiForSheetsSpec.js index 223e5f64..4d25fe71 100644 --- a/spec/apiForSheetsSpec.js +++ b/spec/apiForSheetsSpec.js @@ -30,7 +30,6 @@ const checkCriticalPullRequestModule = require('../lib/checkCriticalPullRequest'); const checkPullRequestTemplateModule = require('../lib/checkPullRequestTemplate'); -const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); const { google } = require('googleapis'); const { OAuth2Client } = require('google-auth-library'); @@ -60,7 +59,6 @@ describe('Api For Sheets Module', () => { .and.callFake(() => { }); spyOn(checkPullRequestTemplateModule, 'checkTemplate') .and.callFake(() => { }); - spyOn(newCodeOwnerModule, 'checkForNewCodeowner').and.callFake(() => { }); github = { issues: { diff --git a/spec/checkCriticalPullRequestSpec.js b/spec/checkCriticalPullRequestSpec.js index 859d7c10..ee6182f7 100644 --- a/spec/checkCriticalPullRequestSpec.js +++ b/spec/checkCriticalPullRequestSpec.js @@ -29,7 +29,6 @@ const checkCriticalPullRequestModule = require('../lib/checkCriticalPullRequest'); const checkPullRequestTemplateModule = require('../lib/checkPullRequestTemplate'); -const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); const scheduler = require('../lib/scheduler'); let payloadData = JSON.parse( @@ -200,7 +199,6 @@ describe('Critical Pull Request Spec', () => { spyOn( checkPullRequestTemplateModule, 'checkTemplate' ).and.callFake(() => { }); - spyOn(newCodeOwnerModule, 'checkForNewCodeowner').and.callFake(() => { }); spyOn( checkCriticalPullRequestModule, 'checkIfPRAffectsDatastoreLayer' diff --git a/spec/checkMergeConflictSpec.js b/spec/checkMergeConflictSpec.js index 59c0ad53..774adc71 100644 --- a/spec/checkMergeConflictSpec.js +++ b/spec/checkMergeConflictSpec.js @@ -20,7 +20,6 @@ const checkMergeConflictModule = require('../lib/checkMergeConflicts'); const scheduler = require('../lib/scheduler'); const checkPullRequestJobModule = require('../lib/checkPullRequestJob'); const checkCronJobModule = require('../lib/checkNewCronJobs'); -const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); const checkCriticalPullRequestModule = require( '../lib/checkCriticalPullRequest' ); @@ -78,7 +77,6 @@ describe('Merge Conflict Check', () => { checkPullRequestJobModule, 'checkForModificationsToFiles' ).and.callFake(() => { }); spyOn(checkCronJobModule, 'checkForNewCronJob').and.callFake(() => { }); - spyOn(newCodeOwnerModule, 'checkForNewCodeowner').and.callFake(() => { }); spyOn( checkCriticalPullRequestModule, 'checkIfPRAffectsDatastoreLayer' diff --git a/spec/checkNewCronJobSpec.js b/spec/checkNewCronJobSpec.js index 4bc32e63..40535586 100644 --- a/spec/checkNewCronJobSpec.js +++ b/spec/checkNewCronJobSpec.js @@ -26,7 +26,6 @@ const checkCriticalPullRequestModule = require('../lib/checkCriticalPullRequest'); const checkPullRequestTemplateModule = require('../lib/checkPullRequestTemplate'); -const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); const scheduler = require('../lib/scheduler'); const { JOBS_AND_FEATURES_TESTING_WIKI_LINK } = require('../lib/utils'); let payloadData = JSON.parse( @@ -209,7 +208,6 @@ describe('Cron Job Spec', () => { spyOn( checkPullRequestTemplateModule, 'checkTemplate' ).and.callFake(() => { }); - spyOn(newCodeOwnerModule, 'checkForNewCodeowner').and.callFake(() => { }); }); describe('When a new cron job is added in a pull request', () => { diff --git a/spec/checkPullRequestBranchSpec.js b/spec/checkPullRequestBranchSpec.js index 81efeb9a..0ffe5a4f 100644 --- a/spec/checkPullRequestBranchSpec.js +++ b/spec/checkPullRequestBranchSpec.js @@ -26,7 +26,6 @@ const checkCriticalPullRequestModule = require('../lib/checkCriticalPullRequest'); const checkPullRequestTemplateModule = require('../lib/checkPullRequestTemplate'); -const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); describe('Pull Request Branch Check', () => { /** @@ -56,7 +55,6 @@ describe('Pull Request Branch Check', () => { .and.callFake(() => { }); spyOn(checkPullRequestTemplateModule, 'checkTemplate') .and.callFake(() => { }); - spyOn(newCodeOwnerModule, 'checkForNewCodeowner').and.callFake(() => { }); github = { issues: { diff --git a/spec/checkPullRequestJobSpec.js b/spec/checkPullRequestJobSpec.js index 51253e9b..81398074 100644 --- a/spec/checkPullRequestJobSpec.js +++ b/spec/checkPullRequestJobSpec.js @@ -23,7 +23,6 @@ const checkCriticalPullRequestModule = require('../lib/checkCriticalPullRequest'); const checkPullRequestTemplateModule = require('../lib/checkPullRequestTemplate'); -const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); const scheduler = require('../lib/scheduler'); const checkCronJobModule = require('../lib/checkNewCronJobs'); const { JOBS_AND_FEATURES_TESTING_WIKI_LINK } = require('../lib/utils'); @@ -443,7 +442,6 @@ describe('Pull Request Job Spec', () => { spyOn( checkPullRequestTemplateModule, 'checkTemplate' ).and.callFake(() => { }); - spyOn(newCodeOwnerModule, 'checkForNewCodeowner').and.callFake(() => { }); }); describe('When a new job file is created in a pull request', () => { diff --git a/spec/checkPullRequestTemplateSpec.js b/spec/checkPullRequestTemplateSpec.js index 6b60548a..4a438a95 100644 --- a/spec/checkPullRequestTemplateSpec.js +++ b/spec/checkPullRequestTemplateSpec.js @@ -27,7 +27,6 @@ const checkPullRequestLabelsModule = require('../lib/checkPullRequestLabels'); const checkPullRequestBranchModule = require('../lib/checkPullRequestBranch'); const checkCriticalPullRequestModule = require('../lib/checkCriticalPullRequest'); -const newCodeOwnerModule = require('../lib/checkForNewCodeowner'); const scheduler = require('../lib/scheduler'); const payloadData = JSON.parse( JSON.stringify(require('../fixtures/pullRequestPayload.json')) @@ -300,7 +299,6 @@ describe('Pull Request Template', () => { ).and.callFake(() => { }); spyOn(checkPullRequestBranchModule, 'checkBranch').and.callFake(() => { }); spyOn(checkPullRequestTemplateModule, 'checkTemplate').and.callThrough(); - spyOn(newCodeOwnerModule, 'checkForNewCodeowner').and.callFake(() => { }); }); describe('when pull request with invalid template is created', () => { From 8b0594faa8c352222a528c6d8b780a9c84792460 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 15:21:40 +0200 Subject: [PATCH 13/18] Tests --- spec/checkPullRequestJobSpec.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/spec/checkPullRequestJobSpec.js b/spec/checkPullRequestJobSpec.js index 81398074..79f22fcd 100644 --- a/spec/checkPullRequestJobSpec.js +++ b/spec/checkPullRequestJobSpec.js @@ -563,9 +563,8 @@ describe('Pull Request Job Spec', () => { issue_number: payloadData.payload.pull_request.number, body: 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, ' + - 'it adds new jobs. The jobs are ' + firstJobNameLink + - ', ' + secondJobNameLink + '.' + newLineFeed + 'Also @' + - author + ', please make sure to fill in the ' + + 'it modifies files in jobs or platform folders.' + newLineFeed + + 'Also @' + author + ', please make sure to fill in the ' + formText + ' for the new jobs to be tested on the backup server. ' + 'This PR can be merged only after the test run is successful. ' + 'Please refer to ' + wikiLinkText + ' for details.' + From 34df8c9dbab1a87311d26636d703258420f120bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 15:22:59 +0200 Subject: [PATCH 14/18] Tests --- spec/checkNewCronJobSpec.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/checkNewCronJobSpec.js b/spec/checkNewCronJobSpec.js index 40535586..5f22df51 100644 --- a/spec/checkNewCronJobSpec.js +++ b/spec/checkNewCronJobSpec.js @@ -199,7 +199,8 @@ describe('Cron Job Spec', () => { spyOn(app, 'auth').and.resolveTo(github); spyOn(checkCronJobModule, 'checkForNewCronJob').and.callThrough(); spyOn( - checkPullRequestJobModule, 'checkForNewJob').and.callFake(() => { }); + checkPullRequestJobModule, 'checkForModificationsToFiles' + ).and.callFake(() => { }); spyOn(apiForSheetsModule, 'checkClaStatus').and.callFake(() => { }); spyOn( checkCriticalPullRequestModule, 'checkIfPRAffectsDatastoreLayer' From c33f7150a09263b9e8d9d51c70c6b2135dc8eed1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 15:26:41 +0200 Subject: [PATCH 15/18] Tests --- spec/checkPullRequestJobSpec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/checkPullRequestJobSpec.js b/spec/checkPullRequestJobSpec.js index 79f22fcd..a4dc7303 100644 --- a/spec/checkPullRequestJobSpec.js +++ b/spec/checkPullRequestJobSpec.js @@ -564,8 +564,8 @@ describe('Pull Request Job Spec', () => { body: 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, ' + 'it modifies files in jobs or platform folders.' + newLineFeed + - 'Also @' + author + ', please make sure to fill in the ' + - formText + ' for the new jobs to be tested on the backup server. ' + + 'Also @' + author + ', please make sure to fill in the ' + formText + + ' for the new job or feature to be tested on the backup server. ' + 'This PR can be merged only after the test run is successful. ' + 'Please refer to ' + wikiLinkText + ' for details.' + newLineFeed + 'Thanks!', From e9445152ce59b2b89be0a20a222eac4a574e20bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 15:43:16 +0200 Subject: [PATCH 16/18] Final touch --- spec/checkPullRequestJobSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/checkPullRequestJobSpec.js b/spec/checkPullRequestJobSpec.js index a4dc7303..22fe5ef8 100644 --- a/spec/checkPullRequestJobSpec.js +++ b/spec/checkPullRequestJobSpec.js @@ -865,4 +865,4 @@ describe('Pull Request Job Spec', () => { expect(github.issues.createComment).not.toHaveBeenCalled(); }); }); -}); \ No newline at end of file +}); From cb2dc13e618cd38107ffa1ce5c68b4e37abc00bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Fri, 28 Jul 2023 15:49:45 +0200 Subject: [PATCH 17/18] Final touch --- spec/checkPullRequestJobSpec.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/checkPullRequestJobSpec.js b/spec/checkPullRequestJobSpec.js index 22fe5ef8..7d9d14e6 100644 --- a/spec/checkPullRequestJobSpec.js +++ b/spec/checkPullRequestJobSpec.js @@ -638,10 +638,10 @@ describe('Pull Request Job Spec', () => { body: 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this ' + 'PR, it modifies files in jobs or platform folders.' + newLineFeed + - 'Also @' + author + ', please make sure to fill in the ' + formText + - ' for the new job or feature to be tested on the backup server. ' + - 'This PR can be merged only after the test run is successful. ' + - 'Please refer to ' + wikiLinkText + ' for details.' + + 'Also @' + author + ', please make sure to fill in the ' + + formText + ' for the new job or feature to be tested on the ' + + 'backup server. This PR can be merged only after the test run ' + + 'is successful. Please refer to ' + wikiLinkText + ' for details.' + newLineFeed + 'Thanks!', repo: payloadData.payload.repository.name, owner: payloadData.payload.repository.owner.login, @@ -716,9 +716,9 @@ describe('Pull Request Job Spec', () => { body: 'Hi @vojtechjelinek, @DubeySandeep, @kevintab95, PTAL at this PR, ' + 'it modifies files in jobs or platform folders.' + newLineFeed + - 'Also @' + author + ', please make sure to ' + - 'fill in the ' + formText + ' for the new job or feature to be ' + - 'tested on the backup server. This PR can be merged only after the test ' + + 'Also @' + author + ', please make sure to ' + 'fill in the ' + + formText + ' for the new job or feature to be tested on the ' + + 'backup server. This PR can be merged only after the test ' + 'run is successful. Please refer to ' + wikiLinkText + ' for details.' + newLineFeed + 'Thanks!', repo: payloadData.payload.repository.name, From a8d0de366a0d3eadf9112c71a27e76732ee17106 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vojt=C4=9Bch=20Jel=C3=ADnek?= Date: Wed, 2 Aug 2023 07:13:14 +0000 Subject: [PATCH 18/18] Address comments --- lib/checkPullRequestJob.js | 4 ++-- spec/checkNewCronJobSpec.js | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/checkPullRequestJob.js b/lib/checkPullRequestJob.js index 053d0674..02aa1fc6 100644 --- a/lib/checkPullRequestJob.js +++ b/lib/checkPullRequestJob.js @@ -28,7 +28,7 @@ const PLATFORM_DIR_PREFIX = 'core/platform/'; /** * @param {string} filename */ -const fileChangesShouldBeVerified = (filename) => { +const shouldFileChangesBeVerified = (filename) => { return ( ( filename.startsWith(JOBS_DIR_PREFIX) || @@ -78,7 +78,7 @@ const checkForModificationsToFiles = async (context) => { // Get new jobs that were created in the PR. const modifiedFilesToBeTested = changedFiles.filter((file) => { - return fileChangesShouldBeVerified(file.filename); + return shouldFileChangesBeVerified(file.filename); }); if (modifiedFilesToBeTested.length > 0) { diff --git a/spec/checkNewCronJobSpec.js b/spec/checkNewCronJobSpec.js index 5f22df51..19003431 100644 --- a/spec/checkNewCronJobSpec.js +++ b/spec/checkNewCronJobSpec.js @@ -516,4 +516,4 @@ describe('Cron Job Spec', () => { expect(github.issues.createComment).not.toHaveBeenCalled(); }); }); -}); \ No newline at end of file +});