From 4c857e7c4b166f9d2cba569c2f95e3aefb595627 Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Mon, 17 Aug 2020 20:06:48 -0400 Subject: [PATCH 01/18] initial xunit-parser work --- packages/api/lib/xunit-parser.js | 91 +++++++++++++++++++++++ packages/api/package.json | 3 +- packages/api/test/fixtures/one_failed.xml | 20 +++++ packages/api/test/xunit-parser-test.js | 25 +++++++ 4 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 packages/api/lib/xunit-parser.js create mode 100644 packages/api/test/fixtures/one_failed.xml create mode 100644 packages/api/test/xunit-parser-test.js diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js new file mode 100644 index 00000000..943f2346 --- /dev/null +++ b/packages/api/lib/xunit-parser.js @@ -0,0 +1,91 @@ +// Copyright 2020 Google LLC +// +// 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 +// +// https://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. + +const xmljs = require('xml-js'); +const {readFileSync} = require('fs'); + + +const parse = (xmlString) => { + const obj = xmljs.xml2js(xmlString, {compact: true}); + const failures = []; + const passes = []; + // Python doesn't always have a top-level testsuites element. + let testsuites = obj['testsuite']; + if (testsuites === undefined) { + testsuites = obj['testsuites']['testsuite']; + } + if (testsuites === undefined) { + return {passes: [], failures: []}; + } + // If there is only one test suite, put it into an array to make it iterable. + if (!Array.isArray(testsuites)) { + testsuites = [testsuites]; + } + for (const suite of testsuites) { + // Ruby doesn't always have _attributes. + const testsuiteName = suite['_attributes'] ? suite['_attributes'].name : undefined; + let testcases = suite['testcase']; + // If there were no tests in the package, continue. + if (testcases === undefined) { + continue; + } + // If there is only one test case, put it into an array to make it iterable. + if (!Array.isArray(testcases)) { + testcases = [testcases]; + } + for (const testcase of testcases) { + console.log("test case!"); + let pkg = testsuiteName; + if ( + !testsuiteName || + testsuiteName === 'pytest' || + testsuiteName === 'Mocha Tests' + ) { + pkg = testcase['_attributes'].classname; + } + // Ignore skipped tests. They didn't pass and they didn't fail. + if (testcase['skipped'] !== undefined) { + continue; + } + const failure = testcase['failure']; + const error = testcase['error']; + if (failure === undefined && error === undefined) { + passes.push({ + package: pkg, + testCase: testcase['_attributes'].name, + passed: true, + }); + continue; + } + // Here we must have a failure or an error. + let log = failure === undefined ? error['_text'] : failure['_text']; + // Java puts its test logs in a CDATA element. + if (log === undefined) { + log = failure['_cdata']; + } + failures.push({ + package: pkg, + testCase: testcase['_attributes'].name, + passed: false, + log, + }); + } + } + return { + passes: passes, + failures: failures, + }; +} + +console.log(parse(readFileSync(require.resolve('../test/fixtures/one_failed.xml'), 'utf8'))); \ No newline at end of file diff --git a/packages/api/package.json b/packages/api/package.json index e2d542af..a07a4eb7 100644 --- a/packages/api/package.json +++ b/packages/api/package.json @@ -33,7 +33,8 @@ "express-session": "^1.17.1", "moment": "^2.27.0", "tap-parser": "^10.0.1", - "uuid": "^8.2.0" + "uuid": "^8.2.0", + "xml-js": "^1.6.11" }, "devDependencies": { "c8": "^7.1.2", diff --git a/packages/api/test/fixtures/one_failed.xml b/packages/api/test/fixtures/one_failed.xml new file mode 100644 index 00000000..3e90b6ed --- /dev/null +++ b/packages/api/test/fixtures/one_failed.xml @@ -0,0 +1,20 @@ + + + + + + + + + + + + + + + +snippet_test.go:242: got output ""; want it to contain "4 Venue 4" snippet_test.go:243: got output ""; want it to contain "19 Venue 19" snippet_test.go:244: got output ""; want it to contain "42 Venue 42" + + + + \ No newline at end of file diff --git a/packages/api/test/xunit-parser-test.js b/packages/api/test/xunit-parser-test.js new file mode 100644 index 00000000..c6d9952b --- /dev/null +++ b/packages/api/test/xunit-parser-test.js @@ -0,0 +1,25 @@ +// Copyright 2020 Google LLC +// +// 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 +// +// https://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. + +const { describe, it, afterEach } = require('mocha'); +const assert = require('assert'); +const parser = require('../lib/xunit-parser'); +const {readFileSync} = require('fs'); + +describe('xunit-parser-test', () => { + it('parses xunit into Javascript object', () => { + const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); + assert.strictEqual(tests.length, 4); + }); +}); \ No newline at end of file From 7b224b511c98aacacb673272d71cca2abc1c6b8e Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Mon, 17 Aug 2020 20:17:15 -0400 Subject: [PATCH 02/18] get rid of pass and fail arrays; just have one array of tests --- packages/api/lib/xunit-parser.js | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js index 943f2346..3cb89cf4 100644 --- a/packages/api/lib/xunit-parser.js +++ b/packages/api/lib/xunit-parser.js @@ -18,15 +18,14 @@ const {readFileSync} = require('fs'); const parse = (xmlString) => { const obj = xmljs.xml2js(xmlString, {compact: true}); - const failures = []; - const passes = []; + const tests = []; // Python doesn't always have a top-level testsuites element. let testsuites = obj['testsuite']; if (testsuites === undefined) { testsuites = obj['testsuites']['testsuite']; } if (testsuites === undefined) { - return {passes: [], failures: []}; + return tests; } // If there is only one test suite, put it into an array to make it iterable. if (!Array.isArray(testsuites)) { @@ -45,7 +44,6 @@ const parse = (xmlString) => { testcases = [testcases]; } for (const testcase of testcases) { - console.log("test case!"); let pkg = testsuiteName; if ( !testsuiteName || @@ -61,7 +59,7 @@ const parse = (xmlString) => { const failure = testcase['failure']; const error = testcase['error']; if (failure === undefined && error === undefined) { - passes.push({ + tests.push({ package: pkg, testCase: testcase['_attributes'].name, passed: true, @@ -74,7 +72,7 @@ const parse = (xmlString) => { if (log === undefined) { log = failure['_cdata']; } - failures.push({ + tests.push({ package: pkg, testCase: testcase['_attributes'].name, passed: false, @@ -82,10 +80,7 @@ const parse = (xmlString) => { }); } } - return { - passes: passes, - failures: failures, - }; + return tests; } console.log(parse(readFileSync(require.resolve('../test/fixtures/one_failed.xml'), 'utf8'))); \ No newline at end of file From 2da2708d0c53c55c213ffef8cbb6b12d95f9bcbe Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Mon, 17 Aug 2020 21:23:05 -0400 Subject: [PATCH 03/18] xunit-parser returns an array of testrun objects --- packages/api/lib/xunit-parser.js | 65 +++++++++++++------------- packages/api/src/post-build.js | 26 +++++++++++ packages/api/test/xunit-parser-test.js | 14 +++--- 3 files changed, 66 insertions(+), 39 deletions(-) diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js index 3cb89cf4..7ecc7faf 100644 --- a/packages/api/lib/xunit-parser.js +++ b/packages/api/lib/xunit-parser.js @@ -13,16 +13,17 @@ // limitations under the License. const xmljs = require('xml-js'); -const {readFileSync} = require('fs'); - +const { readFileSync } = require('fs'); +const TestCaseRun = require('../lib/testrun'); const parse = (xmlString) => { - const obj = xmljs.xml2js(xmlString, {compact: true}); + let count = 1; + const obj = xmljs.xml2js(xmlString, { compact: true }); const tests = []; // Python doesn't always have a top-level testsuites element. - let testsuites = obj['testsuite']; + let testsuites = obj.testsuite; if (testsuites === undefined) { - testsuites = obj['testsuites']['testsuite']; + testsuites = obj.testsuites.testsuite; } if (testsuites === undefined) { return tests; @@ -33,8 +34,8 @@ const parse = (xmlString) => { } for (const suite of testsuites) { // Ruby doesn't always have _attributes. - const testsuiteName = suite['_attributes'] ? suite['_attributes'].name : undefined; - let testcases = suite['testcase']; + const testsuiteName = suite._attributes ? suite._attributes.name : undefined; + let testcases = suite.testcase; // If there were no tests in the package, continue. if (testcases === undefined) { continue; @@ -50,37 +51,37 @@ const parse = (xmlString) => { testsuiteName === 'pytest' || testsuiteName === 'Mocha Tests' ) { - pkg = testcase['_attributes'].classname; + pkg = testcase._attributes.classname; } // Ignore skipped tests. They didn't pass and they didn't fail. - if (testcase['skipped'] !== undefined) { - continue; - } - const failure = testcase['failure']; - const error = testcase['error']; - if (failure === undefined && error === undefined) { - tests.push({ - package: pkg, - testCase: testcase['_attributes'].name, - passed: true, - }); + if (testcase.skipped !== undefined) { continue; } - // Here we must have a failure or an error. - let log = failure === undefined ? error['_text'] : failure['_text']; - // Java puts its test logs in a CDATA element. - if (log === undefined) { - log = failure['_cdata']; + + const failure = testcase.failure; + const error = testcase.error; + + const testCaseRun = new TestCaseRun((failure === undefined && error === undefined) ? 'ok' : 'not ok', count, testcase._attributes.name); + count++; + + if (failure !== undefined || error !== undefined) { + // Here we must have a failure or an error. + let log = (failure === undefined) ? error._text : failure._text; + // Java puts its test logs in a CDATA element. + if (log === undefined) { + log = failure._cdata; + } + + testCaseRun.failureMessage = log; } - tests.push({ - package: pkg, - testCase: testcase['_attributes'].name, - passed: false, - log, - }); + + tests.push(testCaseRun); + console.log(testCaseRun.display()); } } return tests; -} +}; + +// console.log(parse(readFileSync(require.resolve('../test/fixtures/one_failed.xml'), 'utf8'))); -console.log(parse(readFileSync(require.resolve('../test/fixtures/one_failed.xml'), 'utf8'))); \ No newline at end of file +module.exports = parse; diff --git a/packages/api/src/post-build.js b/packages/api/src/post-build.js index de03a945..8d48c9a7 100644 --- a/packages/api/src/post-build.js +++ b/packages/api/src/post-build.js @@ -264,6 +264,32 @@ class PostBuildHandler { handleError(res, err); } }); + + // endpoint expects the the required buildinfo to be in req.body.metadata to already exist and be properly formatted. + // required keys in the req.body.metadata are the inputs for addBuild in src/add-build.js + this.app.post('/api/build/xml', async (req, res, next) => { + try { + // TODO: Check for private repo, reject if it is one. + // if (req.body.metadata.private) { + // throw new UnauthorizedError('Flaky does not store tests for private repos'); + // } + + // const buildInfo = PostBuildHandler.cleanBuildInfo(req.body.metadata); // Different line. The metadata object is the same as addbuild, already validated + + // req.body.data = await PostBuildHandler.flattenTap(req.body.data); + const testCases = PostBuildHandler.parseTestCases(parsedRaw, req.body.data); + + const isValid = await validateGithub(req.body.metadata.token, decodeURIComponent(req.body.metadata.repoId)); + if (!isValid) { + throw new UnauthorizedError('Must have valid Github Token to post build'); + } + + await addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); + res.send({ message: 'successfully added build' }); + } catch (err) { + handleError(res, err); + } + }); } } diff --git a/packages/api/test/xunit-parser-test.js b/packages/api/test/xunit-parser-test.js index c6d9952b..5cfd89e8 100644 --- a/packages/api/test/xunit-parser-test.js +++ b/packages/api/test/xunit-parser-test.js @@ -15,11 +15,11 @@ const { describe, it, afterEach } = require('mocha'); const assert = require('assert'); const parser = require('../lib/xunit-parser'); -const {readFileSync} = require('fs'); +const { readFileSync } = require('fs'); -describe('xunit-parser-test', () => { - it('parses xunit into Javascript object', () => { - const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); - assert.strictEqual(tests.length, 4); - }); -}); \ No newline at end of file +describe.only('xunit-parser-test', () => { + it('parses xunit into Javascript object', () => { + const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); + assert.strictEqual(tests.length, 4); + }); +}); From 4053c911d659169fcc050db61786fb57311cadb6 Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Mon, 17 Aug 2020 21:33:26 -0400 Subject: [PATCH 04/18] endpoint code almost done through tests --- packages/api/src/post-build.js | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/packages/api/src/post-build.js b/packages/api/src/post-build.js index 8d48c9a7..ee7697e9 100644 --- a/packages/api/src/post-build.js +++ b/packages/api/src/post-build.js @@ -18,7 +18,8 @@ const addBuild = require('../src/add-build'); const TestCaseRun = require('../lib/testrun'); -var Parser = require('tap-parser'); +var tapParser = require('tap-parser'); +const xunitParser = require('../lib/xunit-parser'); const Readable = require('stream').Readable; const firebaseEncode = require('../lib/firebase-encode'); const { InvalidParameterError, UnauthorizedError, handleError } = require('../lib/errors'); @@ -121,7 +122,7 @@ class PostBuildHandler { switch (fileType) { case 'TAP': { var data = []; - var p = new Parser(); + var p = new tapParser(); p.on('result', function (assert) { data.push(assert); @@ -274,15 +275,15 @@ class PostBuildHandler { // throw new UnauthorizedError('Flaky does not store tests for private repos'); // } + //TODO: Metadata stuff // const buildInfo = PostBuildHandler.cleanBuildInfo(req.body.metadata); // Different line. The metadata object is the same as addbuild, already validated + const testCases = xunitParser(req.body.data); - // req.body.data = await PostBuildHandler.flattenTap(req.body.data); - const testCases = PostBuildHandler.parseTestCases(parsedRaw, req.body.data); - - const isValid = await validateGithub(req.body.metadata.token, decodeURIComponent(req.body.metadata.repoId)); - if (!isValid) { - throw new UnauthorizedError('Must have valid Github Token to post build'); - } + //TODO: Data validation + // const isValid = await validateGithub(req.body.metadata.token, decodeURIComponent(req.body.metadata.repoId)); + // if (!isValid) { + // throw new UnauthorizedError('Must have valid Github Token to post build'); + // } await addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); res.send({ message: 'successfully added build' }); From daa47e669a8cb0f339e41956463135c77b2b7247 Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Tue, 18 Aug 2020 10:27:15 -0400 Subject: [PATCH 05/18] add second test which checks values in tests --- packages/api/lib/xunit-parser.js | 20 +++++++++----------- packages/api/src/post-build.js | 10 ++++++---- packages/api/test/xunit-parser-test.js | 16 +++++++++++++--- 3 files changed, 28 insertions(+), 18 deletions(-) diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js index 7ecc7faf..a7643722 100644 --- a/packages/api/lib/xunit-parser.js +++ b/packages/api/lib/xunit-parser.js @@ -13,7 +13,6 @@ // limitations under the License. const xmljs = require('xml-js'); -const { readFileSync } = require('fs'); const TestCaseRun = require('../lib/testrun'); const parse = (xmlString) => { @@ -45,13 +44,13 @@ const parse = (xmlString) => { testcases = [testcases]; } for (const testcase of testcases) { - let pkg = testsuiteName; + // let pkg = testsuiteName; if ( !testsuiteName || testsuiteName === 'pytest' || testsuiteName === 'Mocha Tests' ) { - pkg = testcase._attributes.classname; + // pkg = testcase._attributes.classname; } // Ignore skipped tests. They didn't pass and they didn't fail. if (testcase.skipped !== undefined) { @@ -65,14 +64,13 @@ const parse = (xmlString) => { count++; if (failure !== undefined || error !== undefined) { - // Here we must have a failure or an error. - let log = (failure === undefined) ? error._text : failure._text; - // Java puts its test logs in a CDATA element. - if (log === undefined) { - log = failure._cdata; - } - - testCaseRun.failureMessage = log; + // Here we must have a failure or an error. + let log = (failure === undefined) ? error._text : failure._text; + // Java puts its test logs in a CDATA element. + if (log === undefined) { + log = failure._cdata; + } + testCaseRun.failureMessage = log; } tests.push(testCaseRun); diff --git a/packages/api/src/post-build.js b/packages/api/src/post-build.js index ee7697e9..dce12910 100644 --- a/packages/api/src/post-build.js +++ b/packages/api/src/post-build.js @@ -18,7 +18,7 @@ const addBuild = require('../src/add-build'); const TestCaseRun = require('../lib/testrun'); -var tapParser = require('tap-parser'); +var TapParser = require('tap-parser'); const xunitParser = require('../lib/xunit-parser'); const Readable = require('stream').Readable; const firebaseEncode = require('../lib/firebase-encode'); @@ -122,7 +122,7 @@ class PostBuildHandler { switch (fileType) { case 'TAP': { var data = []; - var p = new tapParser(); + var p = new TapParser(); p.on('result', function (assert) { data.push(assert); @@ -275,11 +275,13 @@ class PostBuildHandler { // throw new UnauthorizedError('Flaky does not store tests for private repos'); // } - //TODO: Metadata stuff + // TODO: Metadata stuff // const buildInfo = PostBuildHandler.cleanBuildInfo(req.body.metadata); // Different line. The metadata object is the same as addbuild, already validated const testCases = xunitParser(req.body.data); - //TODO: Data validation + const buildInfo = []; + + // TODO: Data validation // const isValid = await validateGithub(req.body.metadata.token, decodeURIComponent(req.body.metadata.repoId)); // if (!isValid) { // throw new UnauthorizedError('Must have valid Github Token to post build'); diff --git a/packages/api/test/xunit-parser-test.js b/packages/api/test/xunit-parser-test.js index 5cfd89e8..b5e0dbea 100644 --- a/packages/api/test/xunit-parser-test.js +++ b/packages/api/test/xunit-parser-test.js @@ -12,14 +12,24 @@ // See the License for the specific language governing permissions and // limitations under the License. -const { describe, it, afterEach } = require('mocha'); +const { describe, it } = require('mocha'); const assert = require('assert'); const parser = require('../lib/xunit-parser'); const { readFileSync } = require('fs'); -describe.only('xunit-parser-test', () => { - it('parses xunit into Javascript object', () => { +describe('xunit-parser-test', () => { + it('stores the correct number of tests', () => { const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); assert.strictEqual(tests.length, 4); }); + + it('stores a passing test with the correct values', () => { + const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); + const test = tests[0]; + // + assert(test.successful === true); + assert(test.number === 1); + assert(test.name === 'TestBadFiles'); + assert(test.failureMessage === 'Successful'); + }); }); From 681c37d6de70a98d7350eeb36666d3bb725ac396 Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Tue, 18 Aug 2020 17:49:48 -0400 Subject: [PATCH 06/18] test parsing failed test, name test full path minus github link --- packages/api/lib/xunit-parser.js | 24 +-- packages/api/test/addbuild-getbuild.js | 2 +- packages/api/test/fixtures/empty_results.xml | 2 + .../api/test/fixtures/go_failure_group.xml | 29 ++++ packages/api/test/fixtures/go_skip.xml | 17 ++ .../api/test/fixtures/java_one_failed.xml | 113 ++++++++++++ .../api/test/fixtures/java_one_passed.xml | 63 +++++++ .../test/fixtures/many_failed_same_pkg.xml | 27 +++ packages/api/test/fixtures/no_tests.xml | 8 + packages/api/test/fixtures/node_group.xml | 162 ++++++++++++++++++ .../api/test/fixtures/node_group_pass.xml | 4 + .../api/test/fixtures/node_one_failed.xml | 7 + packages/api/test/fixtures/passed.xml | 14 ++ .../api/test/fixtures/python_one_error.xml | 6 + .../api/test/fixtures/python_one_failed.xml | 12 ++ .../api/test/fixtures/python_one_passed.xml | 4 + .../api/test/fixtures/ruby_one_failed.xml | 23 +++ packages/api/test/xunit-parser-test.js | 134 ++++++++++++++- 18 files changed, 636 insertions(+), 15 deletions(-) create mode 100644 packages/api/test/fixtures/empty_results.xml create mode 100644 packages/api/test/fixtures/go_failure_group.xml create mode 100644 packages/api/test/fixtures/go_skip.xml create mode 100644 packages/api/test/fixtures/java_one_failed.xml create mode 100644 packages/api/test/fixtures/java_one_passed.xml create mode 100644 packages/api/test/fixtures/many_failed_same_pkg.xml create mode 100644 packages/api/test/fixtures/no_tests.xml create mode 100644 packages/api/test/fixtures/node_group.xml create mode 100644 packages/api/test/fixtures/node_group_pass.xml create mode 100644 packages/api/test/fixtures/node_one_failed.xml create mode 100644 packages/api/test/fixtures/passed.xml create mode 100644 packages/api/test/fixtures/python_one_error.xml create mode 100644 packages/api/test/fixtures/python_one_failed.xml create mode 100644 packages/api/test/fixtures/python_one_passed.xml create mode 100644 packages/api/test/fixtures/ruby_one_failed.xml diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js index a7643722..7658d17a 100644 --- a/packages/api/lib/xunit-parser.js +++ b/packages/api/lib/xunit-parser.js @@ -33,7 +33,13 @@ const parse = (xmlString) => { } for (const suite of testsuites) { // Ruby doesn't always have _attributes. - const testsuiteName = suite._attributes ? suite._attributes.name : undefined; + let testsuiteName = suite._attributes ? suite._attributes.name : undefined; + + // Get rid of github.com/orgName/repoName/ + testsuiteName = testsuiteName.substring(testsuiteName.indexOf('/') + 1); + testsuiteName = testsuiteName.substring(testsuiteName.indexOf('/') + 1); + testsuiteName = testsuiteName.substring(testsuiteName.indexOf('/') + 1); + let testcases = suite.testcase; // If there were no tests in the package, continue. if (testcases === undefined) { @@ -43,15 +49,8 @@ const parse = (xmlString) => { if (!Array.isArray(testcases)) { testcases = [testcases]; } + for (const testcase of testcases) { - // let pkg = testsuiteName; - if ( - !testsuiteName || - testsuiteName === 'pytest' || - testsuiteName === 'Mocha Tests' - ) { - // pkg = testcase._attributes.classname; - } // Ignore skipped tests. They didn't pass and they didn't fail. if (testcase.skipped !== undefined) { continue; @@ -60,21 +59,22 @@ const parse = (xmlString) => { const failure = testcase.failure; const error = testcase.error; - const testCaseRun = new TestCaseRun((failure === undefined && error === undefined) ? 'ok' : 'not ok', count, testcase._attributes.name); + const testCaseRun = new TestCaseRun((failure === undefined && error === undefined) ? 'ok' : 'not ok', count, testsuiteName + '/' + testcase._attributes.name); count++; - if (failure !== undefined || error !== undefined) { + if (!testCaseRun.successful) { // Here we must have a failure or an error. let log = (failure === undefined) ? error._text : failure._text; // Java puts its test logs in a CDATA element. if (log === undefined) { log = failure._cdata; } + testCaseRun.failureMessage = log; } tests.push(testCaseRun); - console.log(testCaseRun.display()); + // console.log(testCaseRun.display()); } } return tests; diff --git a/packages/api/test/addbuild-getbuild.js b/packages/api/test/addbuild-getbuild.js index fe8ea0e1..4e617dbb 100644 --- a/packages/api/test/addbuild-getbuild.js +++ b/packages/api/test/addbuild-getbuild.js @@ -102,7 +102,7 @@ const buildInfo = [ buildInfo[2].testCases[0].failureMessage = 'Error message'; buildInfo[2].testCases[1].failureMessage = 'Error message'; -describe.only('Add-Build', () => { +describe('Add-Build', () => { before(async () => { global.headCollection = 'testing/' + TESTING_COLLECTION_BASE + uuidv4() + '/repos'; // random collection name for concurrent testing }); diff --git a/packages/api/test/fixtures/empty_results.xml b/packages/api/test/fixtures/empty_results.xml new file mode 100644 index 00000000..4fe0daf9 --- /dev/null +++ b/packages/api/test/fixtures/empty_results.xml @@ -0,0 +1,2 @@ + + diff --git a/packages/api/test/fixtures/go_failure_group.xml b/packages/api/test/fixtures/go_failure_group.xml new file mode 100644 index 00000000..05d23e9c --- /dev/null +++ b/packages/api/test/fixtures/go_failure_group.xml @@ -0,0 +1,29 @@ + + + + + + + + + + + + + + + + + panic: runtime error: invalid memory address or nil pointer dereference /usr/local/go/src/testing/testing.go:874 +0x3a3 /usr/local/go/src/runtime/panic.go:679 +0x1b2 /go/pkg/mod/cloud.google.com/go/bigquery@v1.3.0/iterator.go:106 +0x37 /tmpfs/src/github/golang-samples/bigquery/snippets/querying/bigquery_query_legacy_large_results.go:59 +0x419 /tmpfs/src/github/golang-samples/bigquery/snippets/querying/integration_test.go:90 +0xa5 /usr/local/go/src/testing/testing.go:909 +0xc9 /usr/local/go/src/testing/testing.go:960 +0x350 + + + + + + + + + + + + diff --git a/packages/api/test/fixtures/go_skip.xml b/packages/api/test/fixtures/go_skip.xml new file mode 100644 index 00000000..52ad9b7c --- /dev/null +++ b/packages/api/test/fixtures/go_skip.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/packages/api/test/fixtures/java_one_failed.xml b/packages/api/test/fixtures/java_one_failed.xml new file mode 100644 index 00000000..c71cfa18 --- /dev/null +++ b/packages/api/test/fixtures/java_one_failed.xml @@ -0,0 +1,113 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + but was: + at org.junit.Assert.fail(Assert.java:89) + at org.junit.Assert.failNotEquals(Assert.java:835) + at org.junit.Assert.assertEquals(Assert.java:120) + at org.junit.Assert.assertEquals(Assert.java:146) + at com.google.cloud.vision.it.ITSystemTest.detectSafeSearchGcsTest(ITSystemTest.java:404) + at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) + at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) + at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) + at java.lang.reflect.Method.invoke(Method.java:498) + at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59) + at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12) + at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56) + at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17) + at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) + at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100) + at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366) + at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103) + at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63) + at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331) + at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79) + at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329) + at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66) + at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293) + at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26) + at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27) + at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) + at org.junit.runners.ParentRunner.run(ParentRunner.java:413) + at org.junit.runners.Suite.runChild(Suite.java:128) + at org.junit.runners.Suite.runChild(Suite.java:27) + at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331) + at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79) + at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329) + at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66) + at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293) + at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) + at org.junit.runners.ParentRunner.run(ParentRunner.java:413) + at org.apache.maven.surefire.junitcore.JUnitCore.run(JUnitCore.java:55) + at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.createRequestAndRun(JUnitCoreWrapper.java:137) + at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.executeEager(JUnitCoreWrapper.java:107) + at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.execute(JUnitCoreWrapper.java:83) + at org.apache.maven.surefire.junitcore.JUnitCoreWrapper.execute(JUnitCoreWrapper.java:75) + at org.apache.maven.surefire.junitcore.JUnitCoreProvider.invoke(JUnitCoreProvider.java:158) + at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:377) + at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:138) + at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:465) + at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:451) +]]> + + + \ No newline at end of file diff --git a/packages/api/test/fixtures/java_one_passed.xml b/packages/api/test/fixtures/java_one_passed.xml new file mode 100644 index 00000000..37d07532 --- /dev/null +++ b/packages/api/test/fixtures/java_one_passed.xml @@ -0,0 +1,63 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/packages/api/test/fixtures/many_failed_same_pkg.xml b/packages/api/test/fixtures/many_failed_same_pkg.xml new file mode 100644 index 00000000..d6993408 --- /dev/null +++ b/packages/api/test/fixtures/many_failed_same_pkg.xml @@ -0,0 +1,27 @@ + + + + + + + + + + + + + + + main_test.go:234: failed to create bucket ("golang-samples-tests-8-storage-buckets-tests"): Post https://storage.googleapis.com/storage/v1/b?alt=json&prettyPrint=false&project=golang-samples-tests-8: read tcp 10.142.0.112:33618->108.177.12.128:443: read: connection reset by peer + + + main_test.go:242: failed to enable uniform bucket-level access ("golang-samples-tests-8-storage-buckets-tests"): googleapi: Error 404: Not Found, notFound + + + + + + + + + diff --git a/packages/api/test/fixtures/no_tests.xml b/packages/api/test/fixtures/no_tests.xml new file mode 100644 index 00000000..162edf53 --- /dev/null +++ b/packages/api/test/fixtures/no_tests.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/packages/api/test/fixtures/node_group.xml b/packages/api/test/fixtures/node_group.xml new file mode 100644 index 00000000..52c7def6 --- /dev/null +++ b/packages/api/test/fixtures/node_group.xml @@ -0,0 +1,162 @@ + + + + + expected 'Deleted individual rows in Albums.\n5 records deleted from Singers.\n2 records deleted from Singers.\n0 records deleted from Singers.\n' to include '3 records deleted from Singers.' + AssertionError: expected 'Deleted individual rows in Albums.\n5 records deleted from Singers.\n2 records deleted from Singers.\n0 records deleted from Singers.\n' to include '3 records deleted from Singers.' + at Context.it (system-test/spanner.test.js:198:12) + expected '' to match /SingerId: 1, AlbumId: 1, AlbumTitle: Total Junk/ + AssertionError: expected '' to match /SingerId: 1, AlbumId: 1, AlbumTitle: Total Junk/ + at Context.it (system-test/spanner.test.js:210:12) + expected '' to match /SingerId: 1, AlbumId: 1, AlbumTitle: Total Junk/ + AssertionError: expected '' to match /SingerId: 1, AlbumId: 1, AlbumTitle: Total Junk/ + at Context.it (system-test/spanner.test.js:218:12) + + expected '' to match /Updated data\./ + AssertionError: expected '' to match /Updated data\./ + at Context.it (system-test/spanner.test.js:235:12) + expected '' to match /SingerId: 1, AlbumId: 1, AlbumTitle: Total Junk, MarketingBudget: 100000/ + AssertionError: expected '' to match /SingerId: 1, AlbumId: 1, AlbumTitle: Total Junk, MarketingBudget: 100000/ + at Context.it (system-test/spanner.test.js:246:12) + expected '' to match /SingerId: 1, AlbumId: 1, MarketingBudget: 100000/ + AssertionError: expected '' to match /SingerId: 1, AlbumId: 1, MarketingBudget: 100000/ + at Context.it (system-test/spanner.test.js:261:12) + + + expected '' to match /AlbumId: 2, AlbumTitle: Go, Go, Go, MarketingBudget:/ + AssertionError: expected '' to match /AlbumId: 2, AlbumTitle: Go, Go, Go, MarketingBudget:/ + at Context.it (system-test/spanner.test.js:288:12) + expected '' to match /AlbumId: 1, AlbumTitle: Total Junk, MarketingBudget:/ + AssertionError: expected '' to match /AlbumId: 1, AlbumTitle: Total Junk, MarketingBudget:/ + at Context.it (system-test/spanner.test.js:302:12) + expected '' to match /AlbumId: 1, AlbumTitle: Total Junk/ + AssertionError: expected '' to match /AlbumId: 1, AlbumTitle: Total Junk/ + at Context.it (system-test/spanner.test.js:317:12) + expected '' to match /AlbumId: 1, AlbumTitle: Total Junk/ + AssertionError: expected '' to match /AlbumId: 1, AlbumTitle: Total Junk/ + at Context.it (system-test/spanner.test.js:325:12) + expected '' to match /AlbumId: 2, AlbumTitle: Forever Hold your Peace, MarketingBudget:/ + AssertionError: expected '' to match /AlbumId: 2, AlbumTitle: Forever Hold your Peace, MarketingBudget:/ + at Context.it (system-test/spanner.test.js:333:12) + expected '' to match /AlbumId: 2, AlbumTitle: Forever Hold your Peace, MarketingBudget:/ + AssertionError: expected '' to match /AlbumId: 2, AlbumTitle: Forever Hold your Peace, MarketingBudget:/ + at Context.it (system-test/spanner.test.js:344:12) + expected 'Successfully executed read-only transaction.\n' to match /SingerId: 1, AlbumId: 1, AlbumTitle: Total Junk/ + AssertionError: expected 'Successfully executed read-only transaction.\n' to match /SingerId: 1, AlbumId: 1, AlbumTitle: Total Junk/ + at Context.it (system-test/spanner.test.js:355:12) + expected '' to match /The first album's marketing budget: 100000/ + AssertionError: expected '' to match /The first album's marketing budget: 100000/ + at Context.it (system-test/spanner.test.js:364:12) + + + + expected '' to match /Updated data\./ + AssertionError: expected '' to match /Updated data\./ + at Context.it (system-test/spanner.test.js:426:12) + expected '' to match /SingerId: 1, AlbumId: 1, MarketingBudget: 1000000, LastUpdateTime:/ + AssertionError: expected '' to match /SingerId: 1, AlbumId: 1, MarketingBudget: 1000000, LastUpdateTime:/ + at Context.it (system-test/spanner.test.js:434:12) + + expected '' to match /Inserted data\./ + AssertionError: expected '' to match /Inserted data\./ + at Context.it (system-test/spanner.test.js:465:12) + expected '' to match /SingerId: 1, VenueId: 4, EventDate:/ + AssertionError: expected '' to match /SingerId: 1, VenueId: 4, EventDate:/ + at Context.it (system-test/spanner.test.js:473:12) + + + + + + + expected 'Successfully updated 0 record.\n' to match /Successfully updated 1 record/ + AssertionError: expected 'Successfully updated 0 record.\n' to match /Successfully updated 1 record/ + at Context.it (system-test/spanner.test.js:536:12) + expected 'Successfully deleted 0 record.\n' to match /Successfully deleted 1 record\./ + AssertionError: expected 'Successfully deleted 0 record.\n' to match /Successfully deleted 1 record\./ + at Context.it (system-test/spanner.test.js:544:12) + expected 'Successfully updated 0 records.\n' to match /Successfully updated 2 records/ + AssertionError: expected 'Successfully updated 0 records.\n' to match /Successfully updated 2 records/ + at Context.it (system-test/spanner.test.js:552:12) + + + + + expected '' to match /Successfully executed read-write transaction using DML to transfer 200000 from Album 2 to Album 1/ + AssertionError: expected '' to match /Successfully executed read-write transaction using DML to transfer 200000 from Album 2 to Album 1/ + at Context.it (system-test/spanner.test.js:592:12) + expected 'Successfully updated 0 records.\n' to match /Successfully updated 3 records/ + AssertionError: expected 'Successfully updated 0 records.\n' to match /Successfully updated 3 records/ + at Context.it (system-test/spanner.test.js:603:12) + + Command failed: node dml.js updateUsingBatchDml test-instance-1591914381325 test-database-1591914381325 long-door-651 + ERROR: { Error: Parent row for row [1,3] in table Albums is missing. Row cannot be written. + at Immediate.request (/tmpfs/src/github/nodejs-spanner/build/src/transaction.js:1082:31) + at runCallback (timers.js:706:11) + at tryOnImmediate (timers.js:676:5) + at processImmediate (timers.js:658:5) + code: 5, + metadata: Metadata { internalRepr: Map {}, options: {} }, + rowCounts: [] } + dml.js updateUsingBatchDml <instanceName> <databaseName> <projectId> + + Insert and Update records using Batch DML. + + Options: + --version Show version number [boolean] + --help Show help [boolean] + + { Error: Parent row for row [1,3] in table Albums is missing. Row cannot be written. + at Immediate.request (/tmpfs/src/github/nodejs-spanner/build/src/transaction.js:1082:31) + at runCallback (timers.js:706:11) + at tryOnImmediate (timers.js:676:5) + at processImmediate (timers.js:658:5) + code: 5, + metadata: Metadata { internalRepr: Map {}, options: {} }, + rowCounts: [] } + + Error: Command failed: node dml.js updateUsingBatchDml test-instance-1591914381325 test-database-1591914381325 long-door-651 + ERROR: { Error: Parent row for row [1,3] in table Albums is missing. Row cannot be written. + at Immediate.request (/tmpfs/src/github/nodejs-spanner/build/src/transaction.js:1082:31) + code: 5, + metadata: Metadata { internalRepr: Map {}, options: {} }, + rowCounts: [] } + dml.js updateUsingBatchDml <instanceName> <databaseName> <projectId> + + Insert and Update records using Batch DML. + + Options: + --version Show version number [boolean] + --help Show help [boolean] + + { Error: Parent row for row [1,3] in table Albums is missing. Row cannot be written. + at Immediate.request (/tmpfs/src/github/nodejs-spanner/build/src/transaction.js:1082:31) + code: 5, + metadata: Metadata { internalRepr: Map {}, options: {} }, + rowCounts: [] } + + at checkExecSyncError (child_process.js:629:11) + at Object.execSync (child_process.js:666:13) + at execSync (system-test/spanner.test.js:23:28) + at Context.it (system-test/spanner.test.js:616:20) + + + + + + + + + + + + + + + + + + + + + diff --git a/packages/api/test/fixtures/node_group_pass.xml b/packages/api/test/fixtures/node_group_pass.xml new file mode 100644 index 00000000..bab1c565 --- /dev/null +++ b/packages/api/test/fixtures/node_group_pass.xml @@ -0,0 +1,4 @@ + + + + diff --git a/packages/api/test/fixtures/node_one_failed.xml b/packages/api/test/fixtures/node_one_failed.xml new file mode 100644 index 00000000..628a4b10 --- /dev/null +++ b/packages/api/test/fixtures/node_one_failed.xml @@ -0,0 +1,7 @@ + + + + expected 'Deleted individual rows in Albums.\n5 records deleted from Singers.\n2 records deleted from Singers.\n0 records deleted from Singers.\n' to include '3 records deleted from Singers.' + AssertionError: expected 'Deleted individual rows in Albums.\n5 records deleted from Singers.\n2 records deleted from Singers.\n0 records deleted from Singers.\n' to include '3 records deleted from Singers.' + at Context.it (system-test/spanner.test.js:198:12) + diff --git a/packages/api/test/fixtures/passed.xml b/packages/api/test/fixtures/passed.xml new file mode 100644 index 00000000..f969bc3e --- /dev/null +++ b/packages/api/test/fixtures/passed.xml @@ -0,0 +1,14 @@ + + + + + + + + + + + + + + diff --git a/packages/api/test/fixtures/python_one_error.xml b/packages/api/test/fixtures/python_one_error.xml new file mode 100644 index 00000000..f06e78a7 --- /dev/null +++ b/packages/api/test/fixtures/python_one_error.xml @@ -0,0 +1,6 @@ +Traceback (most recent call last): + File "/workspace/memorystore/redis/cloud_run_deployment/e2e_test.py", line 70, in services + subprocess.run( + File "/usr/local/lib/python3.8/subprocess.py", line 512, in run + raise CalledProcessError(retcode, process.args, +subprocess.CalledProcessError: Command '['gcloud', 'redis', 'instances', 'create', 'test-instance-44d74c74c5', '--region=us-central1', '--network', 'test-network-44d74c74c5', '--project', 'python-docs-samples-tests']' returned non-zero exit status 1. \ No newline at end of file diff --git a/packages/api/test/fixtures/python_one_failed.xml b/packages/api/test/fixtures/python_one_failed.xml new file mode 100644 index 00000000..e967ed68 --- /dev/null +++ b/packages/api/test/fixtures/python_one_failed.xml @@ -0,0 +1,12 @@ + + + + + +Traceback (most recent call last): + File "/tmpfs/src/github/python-docs-samples/appengine/flexible/datastore/main_test.py", line 22, in test_index + ... + + + + diff --git a/packages/api/test/fixtures/python_one_passed.xml b/packages/api/test/fixtures/python_one_passed.xml new file mode 100644 index 00000000..a5afbc82 --- /dev/null +++ b/packages/api/test/fixtures/python_one_passed.xml @@ -0,0 +1,4 @@ + + + + diff --git a/packages/api/test/fixtures/ruby_one_failed.xml b/packages/api/test/fixtures/ruby_one_failed.xml new file mode 100644 index 00000000..2300e410 --- /dev/null +++ b/packages/api/test/fixtures/ruby_one_failed.xml @@ -0,0 +1,23 @@ + + + + + + + + + + +/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/assertions.rb:183:in `assert' +/tmpfs/src/github/ruby-docs-samples/logging/acceptance/sample_test.rb:126:in `block (3 levels) in <top (required)>' +/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/test.rb:98:in `block (3 levels) in run' +/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/test.rb:195:in `capture_exceptions' +/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/test.rb:95:in `block (2 levels) in run' +/usr/local/bundle/gems/minitest-5.14.1/lib/minitest.rb:272:in `time_it' +/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/test.rb:94:in `block in run' +/usr/local/bundle/gems/minitest-5.14.1/lib/minitest.rb:367:in `on_signal' +/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/test.rb:211:in `with_info_handler' +/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/test.rb:93:in `run' +/usr/local/bundle/gems/minitest-5.14.1/lib/minitest.rb:1029:in `run_one_method' +/usr/local/bundle/gems/minitest-5.14.1/lib/minitest/parallel.rb:33:in `block (2 levels) in start' + \ No newline at end of file diff --git a/packages/api/test/xunit-parser-test.js b/packages/api/test/xunit-parser-test.js index b5e0dbea..f6755ec4 100644 --- a/packages/api/test/xunit-parser-test.js +++ b/packages/api/test/xunit-parser-test.js @@ -17,7 +17,7 @@ const assert = require('assert'); const parser = require('../lib/xunit-parser'); const { readFileSync } = require('fs'); -describe('xunit-parser-test', () => { +describe.only('xunit-parser-test', () => { it('stores the correct number of tests', () => { const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); assert.strictEqual(tests.length, 4); @@ -26,10 +26,140 @@ describe('xunit-parser-test', () => { it('stores a passing test with the correct values', () => { const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); const test = tests[0]; - // + assert(test.successful === true); assert(test.number === 1); assert(test.name === 'TestBadFiles'); assert(test.failureMessage === 'Successful'); }); + + it('stores a failing test with the correct values', () => { + const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); + const test = tests[3]; + + assert.strictEqual(test.successful, false); + assert.strictEqual(test.number, 4); + assert.strictEqual(test.name, 'spanner/spanner_snippets/TestSample'); + assert.strictEqual(test.failureMessage, '\nsnippet_test.go:242: got output ""; want it to contain "4 Venue 4" snippet_test.go:243: got output ""; want it to contain "19 Venue 19" snippet_test.go:244: got output ""; want it to contain "42 Venue 42"\n'); + }); + + it('finds one failure', () => { + const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); + expect(results).to.eql({ + failures: [ + { + package: + 'github.com/GoogleCloudPlatform/golang-samples/spanner/spanner_snippets', + testCase: 'TestSample', + passed: false, + log: + '\nsnippet_test.go:242: got output ""; want it to contain "4 Venue 4" snippet_test.go:243: got output ""; want it to contain "19 Venue 19" snippet_test.go:244: got output ""; want it to contain "42 Venue 42"\n' + } + ], + passes: [ + { + package: 'github.com/GoogleCloudPlatform/golang-samples', + testCase: 'TestBadFiles', + passed: true + }, + { + package: 'github.com/GoogleCloudPlatform/golang-samples', + testCase: 'TestLicense', + passed: true + }, + { + package: 'github.com/GoogleCloudPlatform/golang-samples', + testCase: 'TestRegionTags', + passed: true + } + ] + }); + }); + + it('finds many failures in one package', () => { + const tests = parser(readFileSync(require.resolve('./fixtures/many_failed_same_pkg.xml'), 'utf8')); + expect(results).to.eql({ + failures: [ + { + package: + 'github.com/GoogleCloudPlatform/golang-samples/storage/buckets', + testCase: 'TestBucketLock', + passed: false, + log: + 'main_test.go:234: failed to create bucket ("golang-samples-tests-8-storage-buckets-tests"): Post https://storage.googleapis.com/storage/v1/b?alt=json&prettyPrint=false&project=golang-samples-tests-8: read tcp 10.142.0.112:33618->108.177.12.128:443: read: connection reset by peer' + }, + { + package: + 'github.com/GoogleCloudPlatform/golang-samples/storage/buckets', + testCase: 'TestUniformBucketLevelAccess', + passed: false, + log: + 'main_test.go:242: failed to enable uniform bucket-level access ("golang-samples-tests-8-storage-buckets-tests"): googleapi: Error 404: Not Found, notFound' + } + ], + passes: [ + { + package: 'github.com/GoogleCloudPlatform/golang-samples', + testCase: 'TestBadFiles', + passed: true + }, + { + package: + 'github.com/GoogleCloudPlatform/golang-samples/storage/buckets', + testCase: 'TestCreate', + passed: true + }, + { + package: + 'github.com/GoogleCloudPlatform/golang-samples/storage/gcsupload', + testCase: 'TestUpload', + passed: true + } + ] + }); + }); + + it('finds no failures in a successful log', () => { + const tests = parser(readFileSync(require.resolve('./fixtures/passed.xml'), 'utf8')); + const results = findTestResults(input); + expect(results).to.eql({ + failures: [], + passes: [ + { + package: 'github.com/GoogleCloudPlatform/golang-samples', + testCase: 'TestBadFiles', + passed: true + }, + { + package: + 'github.com/GoogleCloudPlatform/golang-samples/appengine/go11x/helloworld', + testCase: 'TestIndexHandler', + passed: true + } + ] + }); + }); + + it('handles an empty testsuites', () => { + const tests = parser(readFileSync(require.resolve('./fixtures/empty_results.xml'), 'utf8')); + expect(results).to.eql({ + passes: [], + failures: [] + }); + }); + + it('ignores skipped tests', () => { + const tests = parser(readFileSync(require.resolve('./fixtures/go_skip.xml'), 'utf8')); + const results = findTestResults(input); + expect(results).to.eql({ + failures: [], + passes: [ + { + package: 'github.com/GoogleCloudPlatform/golang-samples', + testCase: 'TestLicense', + passed: true + } + ] + }); + }); }); From b949655f19ae89af0d70607a6a7aa3f26f8af12a Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Tue, 18 Aug 2020 18:08:52 -0400 Subject: [PATCH 07/18] linting --- packages/api/lib/xunit-parser.js | 9 +- packages/api/test/xunit-parser-test.js | 132 ++----------------------- 2 files changed, 15 insertions(+), 126 deletions(-) diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js index 7658d17a..e1b45622 100644 --- a/packages/api/lib/xunit-parser.js +++ b/packages/api/lib/xunit-parser.js @@ -38,7 +38,11 @@ const parse = (xmlString) => { // Get rid of github.com/orgName/repoName/ testsuiteName = testsuiteName.substring(testsuiteName.indexOf('/') + 1); testsuiteName = testsuiteName.substring(testsuiteName.indexOf('/') + 1); - testsuiteName = testsuiteName.substring(testsuiteName.indexOf('/') + 1); + const index = testsuiteName.indexOf('/'); + if (index !== -1) { testsuiteName = testsuiteName.substring(index + 1); } else { + // There is nothing past the repo url + testsuiteName = ''; + } let testcases = suite.testcase; // If there were no tests in the package, continue. @@ -59,7 +63,7 @@ const parse = (xmlString) => { const failure = testcase.failure; const error = testcase.error; - const testCaseRun = new TestCaseRun((failure === undefined && error === undefined) ? 'ok' : 'not ok', count, testsuiteName + '/' + testcase._attributes.name); + const testCaseRun = new TestCaseRun((failure === undefined && error === undefined) ? 'ok' : 'not ok', count, (testsuiteName !== '') ? testsuiteName + '/' + testcase._attributes.name : testcase._attributes.name); count++; if (!testCaseRun.successful) { @@ -74,7 +78,6 @@ const parse = (xmlString) => { } tests.push(testCaseRun); - // console.log(testCaseRun.display()); } } return tests; diff --git a/packages/api/test/xunit-parser-test.js b/packages/api/test/xunit-parser-test.js index f6755ec4..30706ed5 100644 --- a/packages/api/test/xunit-parser-test.js +++ b/packages/api/test/xunit-parser-test.js @@ -17,7 +17,7 @@ const assert = require('assert'); const parser = require('../lib/xunit-parser'); const { readFileSync } = require('fs'); -describe.only('xunit-parser-test', () => { +describe('xunit-parser-test', () => { it('stores the correct number of tests', () => { const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); assert.strictEqual(tests.length, 4); @@ -27,139 +27,25 @@ describe.only('xunit-parser-test', () => { const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); const test = tests[0]; - assert(test.successful === true); - assert(test.number === 1); - assert(test.name === 'TestBadFiles'); - assert(test.failureMessage === 'Successful'); + assert.strictEqual(test.successful, true); + assert.strictEqual(test.number, 1); + assert.strictEqual(test.name, 'TestBadFiles'); + assert.strictEqual(test.failureMessage, 'Successful'); }); it('stores a failing test with the correct values', () => { - const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); - const test = tests[3]; - - assert.strictEqual(test.successful, false); + const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); + const test = tests[3]; assert.strictEqual(test.successful, false); assert.strictEqual(test.number, 4); assert.strictEqual(test.name, 'spanner/spanner_snippets/TestSample'); assert.strictEqual(test.failureMessage, '\nsnippet_test.go:242: got output ""; want it to contain "4 Venue 4" snippet_test.go:243: got output ""; want it to contain "19 Venue 19" snippet_test.go:244: got output ""; want it to contain "42 Venue 42"\n'); }); - it('finds one failure', () => { - const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); - expect(results).to.eql({ - failures: [ - { - package: - 'github.com/GoogleCloudPlatform/golang-samples/spanner/spanner_snippets', - testCase: 'TestSample', - passed: false, - log: - '\nsnippet_test.go:242: got output ""; want it to contain "4 Venue 4" snippet_test.go:243: got output ""; want it to contain "19 Venue 19" snippet_test.go:244: got output ""; want it to contain "42 Venue 42"\n' - } - ], - passes: [ - { - package: 'github.com/GoogleCloudPlatform/golang-samples', - testCase: 'TestBadFiles', - passed: true - }, - { - package: 'github.com/GoogleCloudPlatform/golang-samples', - testCase: 'TestLicense', - passed: true - }, - { - package: 'github.com/GoogleCloudPlatform/golang-samples', - testCase: 'TestRegionTags', - passed: true - } - ] - }); - }); - - it('finds many failures in one package', () => { - const tests = parser(readFileSync(require.resolve('./fixtures/many_failed_same_pkg.xml'), 'utf8')); - expect(results).to.eql({ - failures: [ - { - package: - 'github.com/GoogleCloudPlatform/golang-samples/storage/buckets', - testCase: 'TestBucketLock', - passed: false, - log: - 'main_test.go:234: failed to create bucket ("golang-samples-tests-8-storage-buckets-tests"): Post https://storage.googleapis.com/storage/v1/b?alt=json&prettyPrint=false&project=golang-samples-tests-8: read tcp 10.142.0.112:33618->108.177.12.128:443: read: connection reset by peer' - }, - { - package: - 'github.com/GoogleCloudPlatform/golang-samples/storage/buckets', - testCase: 'TestUniformBucketLevelAccess', - passed: false, - log: - 'main_test.go:242: failed to enable uniform bucket-level access ("golang-samples-tests-8-storage-buckets-tests"): googleapi: Error 404: Not Found, notFound' - } - ], - passes: [ - { - package: 'github.com/GoogleCloudPlatform/golang-samples', - testCase: 'TestBadFiles', - passed: true - }, - { - package: - 'github.com/GoogleCloudPlatform/golang-samples/storage/buckets', - testCase: 'TestCreate', - passed: true - }, - { - package: - 'github.com/GoogleCloudPlatform/golang-samples/storage/gcsupload', - testCase: 'TestUpload', - passed: true - } - ] - }); - }); - - it('finds no failures in a successful log', () => { - const tests = parser(readFileSync(require.resolve('./fixtures/passed.xml'), 'utf8')); - const results = findTestResults(input); - expect(results).to.eql({ - failures: [], - passes: [ - { - package: 'github.com/GoogleCloudPlatform/golang-samples', - testCase: 'TestBadFiles', - passed: true - }, - { - package: - 'github.com/GoogleCloudPlatform/golang-samples/appengine/go11x/helloworld', - testCase: 'TestIndexHandler', - passed: true - } - ] - }); - }); - it('handles an empty testsuites', () => { - const tests = parser(readFileSync(require.resolve('./fixtures/empty_results.xml'), 'utf8')); - expect(results).to.eql({ - passes: [], - failures: [] - }); + const tests = parser(readFileSync(require.resolve('./fixtures/empty_results.xml'), 'utf8')); assert.deepStrictEqual(tests, []); }); it('ignores skipped tests', () => { - const tests = parser(readFileSync(require.resolve('./fixtures/go_skip.xml'), 'utf8')); - const results = findTestResults(input); - expect(results).to.eql({ - failures: [], - passes: [ - { - package: 'github.com/GoogleCloudPlatform/golang-samples', - testCase: 'TestLicense', - passed: true - } - ] - }); + const tests = parser(readFileSync(require.resolve('./fixtures/go_skip.xml'), 'utf8')); assert.strictEqual(tests.length, 1); }); }); From 5a07801ea8ead51c53a5131bb6349b5046923af3 Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Wed, 19 Aug 2020 12:57:18 -0400 Subject: [PATCH 08/18] remove count var from testrun.js --- packages/api/lib/testrun.js | 5 ++--- packages/api/lib/xunit-parser.js | 4 +--- packages/api/src/post-build.js | 2 +- packages/api/test/addbuild-getbuild.js | 20 ++++++++++---------- packages/api/test/post-build-test.js | 2 -- packages/api/test/xunit-parser-test.js | 10 +++++----- 6 files changed, 19 insertions(+), 24 deletions(-) diff --git a/packages/api/lib/testrun.js b/packages/api/lib/testrun.js index bee5e13c..1df0b1b3 100644 --- a/packages/api/lib/testrun.js +++ b/packages/api/lib/testrun.js @@ -15,16 +15,15 @@ const firebaseEncode = require('./firebase-encode'); class TestCaseRun { - constructor (okMessage, number, name) { + constructor (okMessage, name) { this.successful = (okMessage === 'ok'); - this.number = number; this.name = name; this.encodedName = firebaseEncode(this.name); this.failureMessage = 'Successful'; } display () { - return this.number + ', ' + this.name + ', ' + this.time + ', ' + (this.successful ? '1' : '0') + ', ' + this.failureMessage; + return this.name + ', ' + this.time + ', ' + (this.successful ? '1' : '0') + ', ' + this.failureMessage; } } diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js index e1b45622..a550729b 100644 --- a/packages/api/lib/xunit-parser.js +++ b/packages/api/lib/xunit-parser.js @@ -16,7 +16,6 @@ const xmljs = require('xml-js'); const TestCaseRun = require('../lib/testrun'); const parse = (xmlString) => { - let count = 1; const obj = xmljs.xml2js(xmlString, { compact: true }); const tests = []; // Python doesn't always have a top-level testsuites element. @@ -63,8 +62,7 @@ const parse = (xmlString) => { const failure = testcase.failure; const error = testcase.error; - const testCaseRun = new TestCaseRun((failure === undefined && error === undefined) ? 'ok' : 'not ok', count, (testsuiteName !== '') ? testsuiteName + '/' + testcase._attributes.name : testcase._attributes.name); - count++; + const testCaseRun = new TestCaseRun((failure === undefined && error === undefined) ? 'ok' : 'not ok', (testsuiteName !== '') ? testsuiteName + '/' + testcase._attributes.name : testcase._attributes.name); if (!testCaseRun.successful) { // Here we must have a failure or an error. diff --git a/packages/api/src/post-build.js b/packages/api/src/post-build.js index dce12910..5ed34fff 100644 --- a/packages/api/src/post-build.js +++ b/packages/api/src/post-build.js @@ -97,7 +97,7 @@ class PostBuildHandler { if (typeof x.ok !== 'boolean' || !x.id || !x.name) { throw new InvalidParameterError('Missing All Test Case Info'); } - const testcase = new TestCaseRun(x.ok ? 'ok' : 'not ok', x.id, x.name); + const testcase = new TestCaseRun(x.ok ? 'ok' : 'not ok', x.name); // wrap failure message generation in try so still works if ids arent sequential try { diff --git a/packages/api/test/addbuild-getbuild.js b/packages/api/test/addbuild-getbuild.js index 4e617dbb..c91706f0 100644 --- a/packages/api/test/addbuild-getbuild.js +++ b/packages/api/test/addbuild-getbuild.js @@ -47,10 +47,10 @@ const buildInfo = [ }, timestamp: new Date('01/01/2000'), testCases: [ - new TestCaseRun('ok', 1, 'a/1'), - new TestCaseRun('not ok', 2, 'a/2'), - new TestCaseRun('ok', 3, 'a/3'), - new TestCaseRun('not ok', 4, 'a/4') + new TestCaseRun('ok', 'a/1'), + new TestCaseRun('not ok', 'a/2'), + new TestCaseRun('ok', 'a/3'), + new TestCaseRun('not ok', 'a/4') ], description: 'nodejs repository', buildmessage: 'Workflow - 1' @@ -70,8 +70,8 @@ const buildInfo = [ }, timestamp: new Date('01/01/2001'), testCases: [ - new TestCaseRun('ok', 1, 'a/1'), - new TestCaseRun('ok', 2, 'a/2') // this test is now passing + new TestCaseRun('ok', 'a/1'), + new TestCaseRun('ok', 'a/2') // this test is now passing ], description: 'nodejs repository', buildmessage: 'Workflow - 2' @@ -91,8 +91,8 @@ const buildInfo = [ }, timestamp: new Date('01/01/2002'), testCases: [ - new TestCaseRun('not ok', 1, 'a/5'), - new TestCaseRun('not ok', 2, 'a/2') // this test is now failing + new TestCaseRun('not ok', 'a/5'), + new TestCaseRun('not ok', 'a/2') // this test is now failing ], description: 'None', buildmessage: 'Workflow - 1' @@ -434,8 +434,8 @@ describe('Add-Build', () => { }, timestamp: new Date('01/01/2004'), testCases: [ - new TestCaseRun('not ok', 1, 'a/5'), - new TestCaseRun('not ok', 2, 'a/2') // this test is now passing + new TestCaseRun('not ok', 'a/5'), + new TestCaseRun('not ok', 'a/2') // this test is now passing ], description: 'nodejs repository', buildmessage: 'Workflow - 42' diff --git a/packages/api/test/post-build-test.js b/packages/api/test/post-build-test.js index f2bda51a..67621ba0 100644 --- a/packages/api/test/post-build-test.js +++ b/packages/api/test/post-build-test.js @@ -73,12 +73,10 @@ describe('Posting Builds', () => { // two failed assert(!testcases[2].successful); - assert.strictEqual(testcases[2].number, 3); assert(testcases[2].failureMessage.includes('AssertionError')); // five failed assert(!testcases[5].successful); - assert.strictEqual(testcases[5].number, 6); assert(testcases[5].failureMessage.includes('AssertionError')); }); diff --git a/packages/api/test/xunit-parser-test.js b/packages/api/test/xunit-parser-test.js index 30706ed5..77982360 100644 --- a/packages/api/test/xunit-parser-test.js +++ b/packages/api/test/xunit-parser-test.js @@ -28,7 +28,6 @@ describe('xunit-parser-test', () => { const test = tests[0]; assert.strictEqual(test.successful, true); - assert.strictEqual(test.number, 1); assert.strictEqual(test.name, 'TestBadFiles'); assert.strictEqual(test.failureMessage, 'Successful'); }); @@ -36,16 +35,17 @@ describe('xunit-parser-test', () => { it('stores a failing test with the correct values', () => { const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); const test = tests[3]; assert.strictEqual(test.successful, false); - assert.strictEqual(test.number, 4); assert.strictEqual(test.name, 'spanner/spanner_snippets/TestSample'); assert.strictEqual(test.failureMessage, '\nsnippet_test.go:242: got output ""; want it to contain "4 Venue 4" snippet_test.go:243: got output ""; want it to contain "19 Venue 19" snippet_test.go:244: got output ""; want it to contain "42 Venue 42"\n'); }); - it('handles an empty testsuites', () => { - const tests = parser(readFileSync(require.resolve('./fixtures/empty_results.xml'), 'utf8')); assert.deepStrictEqual(tests, []); + it('handles an empty testsuite', () => { + const tests = parser(readFileSync(require.resolve('./fixtures/empty_results.xml'), 'utf8')); + assert.deepStrictEqual(tests, []); }); it('ignores skipped tests', () => { - const tests = parser(readFileSync(require.resolve('./fixtures/go_skip.xml'), 'utf8')); assert.strictEqual(tests.length, 1); + const tests = parser(readFileSync(require.resolve('./fixtures/go_skip.xml'), 'utf8')); + assert.strictEqual(tests.length, 1); }); }); From 70fd1c4da80ce20f192d1c40237e901fd870a120 Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Wed, 19 Aug 2020 13:00:40 -0400 Subject: [PATCH 09/18] const to var --- packages/api/src/post-build.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/api/src/post-build.js b/packages/api/src/post-build.js index 5ed34fff..8d914b89 100644 --- a/packages/api/src/post-build.js +++ b/packages/api/src/post-build.js @@ -18,7 +18,7 @@ const addBuild = require('../src/add-build'); const TestCaseRun = require('../lib/testrun'); -var TapParser = require('tap-parser'); +const TapParser = require('tap-parser'); const xunitParser = require('../lib/xunit-parser'); const Readable = require('stream').Readable; const firebaseEncode = require('../lib/firebase-encode'); From 9631fe2cac19d140be4fc3ba480aa2b8d0caacde Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Thu, 20 Aug 2020 16:47:28 -0400 Subject: [PATCH 10/18] stub out the build parsing --- packages/api/lib/xunit-parser.js | 136 +++++++++++++++---------- packages/api/src/post-build.js | 26 ++--- packages/api/test/xunit-parser-test.js | 10 +- 3 files changed, 96 insertions(+), 76 deletions(-) diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js index a550729b..c17fe64f 100644 --- a/packages/api/lib/xunit-parser.js +++ b/packages/api/lib/xunit-parser.js @@ -15,72 +15,102 @@ const xmljs = require('xml-js'); const TestCaseRun = require('../lib/testrun'); -const parse = (xmlString) => { - const obj = xmljs.xml2js(xmlString, { compact: true }); - const tests = []; - // Python doesn't always have a top-level testsuites element. - let testsuites = obj.testsuite; - if (testsuites === undefined) { - testsuites = obj.testsuites.testsuite; - } - if (testsuites === undefined) { - return tests; - } - // If there is only one test suite, put it into an array to make it iterable. - if (!Array.isArray(testsuites)) { - testsuites = [testsuites]; - } - for (const suite of testsuites) { - // Ruby doesn't always have _attributes. - let testsuiteName = suite._attributes ? suite._attributes.name : undefined; - - // Get rid of github.com/orgName/repoName/ - testsuiteName = testsuiteName.substring(testsuiteName.indexOf('/') + 1); - testsuiteName = testsuiteName.substring(testsuiteName.indexOf('/') + 1); - const index = testsuiteName.indexOf('/'); - if (index !== -1) { testsuiteName = testsuiteName.substring(index + 1); } else { - // There is nothing past the repo url - testsuiteName = ''; +class Parser { + parse (xmlString) { + const obj = xmljs.xml2js(xmlString, { compact: true }); + const tests = []; + // Python doesn't always have a top-level testsuites element. + let testsuites = obj.testsuite; + if (testsuites === undefined) { + testsuites = obj.testsuites.testsuite; } - - let testcases = suite.testcase; - // If there were no tests in the package, continue. - if (testcases === undefined) { - continue; + if (testsuites === undefined) { + return tests; } - // If there is only one test case, put it into an array to make it iterable. - if (!Array.isArray(testcases)) { - testcases = [testcases]; + // If there is only one test suite, put it into an array to make it iterable. + if (!Array.isArray(testsuites)) { + testsuites = [testsuites]; } + for (const suite of testsuites) { + // Ruby doesn't always have _attributes. + let testsuiteName = suite._attributes ? suite._attributes.name : undefined; - for (const testcase of testcases) { - // Ignore skipped tests. They didn't pass and they didn't fail. - if (testcase.skipped !== undefined) { + // Get rid of github.com/orgName/repoName/ + testsuiteName = testsuiteName.substring(testsuiteName.indexOf('/') + 1); + testsuiteName = testsuiteName.substring(testsuiteName.indexOf('/') + 1); + const index = testsuiteName.indexOf('/'); + if (index !== -1) { testsuiteName = testsuiteName.substring(index + 1); } else { + // There is nothing past the repo url + testsuiteName = ''; + } + + let testcases = suite.testcase; + // If there were no tests in the package, continue. + if (testcases === undefined) { continue; } + // If there is only one test case, put it into an array to make it iterable. + if (!Array.isArray(testcases)) { + testcases = [testcases]; + } - const failure = testcase.failure; - const error = testcase.error; + for (const testcase of testcases) { + // Ignore skipped tests. They didn't pass and they didn't fail. + if (testcase.skipped !== undefined) { + continue; + } + + const failure = testcase.failure; + const error = testcase.error; - const testCaseRun = new TestCaseRun((failure === undefined && error === undefined) ? 'ok' : 'not ok', (testsuiteName !== '') ? testsuiteName + '/' + testcase._attributes.name : testcase._attributes.name); + const testCaseRun = new TestCaseRun((failure === undefined && error === undefined) ? 'ok' : 'not ok', (testsuiteName !== '') ? testsuiteName + '/' + testcase._attributes.name : testcase._attributes.name); - if (!testCaseRun.successful) { - // Here we must have a failure or an error. - let log = (failure === undefined) ? error._text : failure._text; - // Java puts its test logs in a CDATA element. - if (log === undefined) { - log = failure._cdata; + if (!testCaseRun.successful) { + // Here we must have a failure or an error. + let log = (failure === undefined) ? error._text : failure._text; + // Java puts its test logs in a CDATA element. + if (log === undefined) { + log = failure._cdata; + } + + testCaseRun.failureMessage = log; } - testCaseRun.failureMessage = log; + tests.push(testCaseRun); } - - tests.push(testCaseRun); } + return tests; } - return tests; -}; -// console.log(parse(readFileSync(require.resolve('../test/fixtures/one_failed.xml'), 'utf8'))); + // // IMPORTANT: All values that will be used as keys in Firestore must be escaped with the firestoreEncode function + // static cleanXunitBuildInfo (metadata) { + // const timestampNumb = Date.parse(metadata.timestamp); + // const timestamp = isNaN(timestampNumb) ? new Date() : new Date(timestampNumb); + + // const returnVal = { + // repoId: firebaseEncode(decodeURIComponent(metadata.repoId)), + // organization: metadata.organization, + // timestamp, + // url: metadata.url, + // environment: PostBuildHandler.cleanEnvironment(metadata), + // buildId: firebaseEncode(decodeURIComponent(metadata.buildId)), + // sha: metadata.sha, + // name: metadata.name, + // description: metadata.description, + // buildmessage: metadata.buildmessage + // }; + + // // validate data + // for (const prop in returnVal) { + // if (!returnVal[prop]) { + // throw new InvalidParameterError('Missing All Build Meta Data Info - ' + prop); + // } + // } -module.exports = parse; + // return returnVal; + // } +} + +// console.log(parse(readFileSync(require.resolve('../test/fixtures/one_failed.xml'), 'utf8'))); +const parserHandler = new Parser(); +module.exports = parserHandler; diff --git a/packages/api/src/post-build.js b/packages/api/src/post-build.js index 8d914b89..09bf5cf4 100644 --- a/packages/api/src/post-build.js +++ b/packages/api/src/post-build.js @@ -170,7 +170,7 @@ class PostBuildHandler { } // IMPORTANT: All values that will be used as keys in Firestore must be escaped with the firestoreEncode function - static cleanBuildInfo (metadata) { + static cleanTapBuildInfo (metadata) { const timestampNumb = Date.parse(metadata.timestamp); const timestamp = isNaN(timestampNumb) ? new Date() : new Date(timestampNumb); @@ -248,7 +248,7 @@ class PostBuildHandler { throw new UnauthorizedError('Flaky does not store tests for private repos'); } - const buildInfo = PostBuildHandler.cleanBuildInfo(req.body.metadata); // Different line. The metadata object is the same as addbuild, already validated + const buildInfo = PostBuildHandler.cleanTapBuildInfo(req.body.metadata); // Different line. The metadata object is the same as addbuild, already validated req.body.data = await PostBuildHandler.flattenTap(req.body.data); const parsedRaw = await PostBuildHandler.parseRawOutput(req.body.data, req.body.type); @@ -270,22 +270,12 @@ class PostBuildHandler { // required keys in the req.body.metadata are the inputs for addBuild in src/add-build.js this.app.post('/api/build/xml', async (req, res, next) => { try { - // TODO: Check for private repo, reject if it is one. - // if (req.body.metadata.private) { - // throw new UnauthorizedError('Flaky does not store tests for private repos'); - // } - - // TODO: Metadata stuff - // const buildInfo = PostBuildHandler.cleanBuildInfo(req.body.metadata); // Different line. The metadata object is the same as addbuild, already validated - const testCases = xunitParser(req.body.data); - - const buildInfo = []; - - // TODO: Data validation - // const isValid = await validateGithub(req.body.metadata.token, decodeURIComponent(req.body.metadata.repoId)); - // if (!isValid) { - // throw new UnauthorizedError('Must have valid Github Token to post build'); - // } + if (req.body.metadata.token !== process.env.buildCopSecret) { + throw new UnauthorizedError('Invalid Secret. Only Google Employees may use this endpoint.'); + } + + const testCases = xunitParser.parse(req.body.data); + const buildInfo = xunitParser.cleanXunitBuildInfo(req.body.metadata); await addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); res.send({ message: 'successfully added build' }); diff --git a/packages/api/test/xunit-parser-test.js b/packages/api/test/xunit-parser-test.js index 77982360..1b7cc3a2 100644 --- a/packages/api/test/xunit-parser-test.js +++ b/packages/api/test/xunit-parser-test.js @@ -19,12 +19,12 @@ const { readFileSync } = require('fs'); describe('xunit-parser-test', () => { it('stores the correct number of tests', () => { - const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); + const tests = parser.parse(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); assert.strictEqual(tests.length, 4); }); it('stores a passing test with the correct values', () => { - const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); + const tests = parser.parse(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); const test = tests[0]; assert.strictEqual(test.successful, true); @@ -33,19 +33,19 @@ describe('xunit-parser-test', () => { }); it('stores a failing test with the correct values', () => { - const tests = parser(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); + const tests = parser.parse(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); const test = tests[3]; assert.strictEqual(test.successful, false); assert.strictEqual(test.name, 'spanner/spanner_snippets/TestSample'); assert.strictEqual(test.failureMessage, '\nsnippet_test.go:242: got output ""; want it to contain "4 Venue 4" snippet_test.go:243: got output ""; want it to contain "19 Venue 19" snippet_test.go:244: got output ""; want it to contain "42 Venue 42"\n'); }); it('handles an empty testsuite', () => { - const tests = parser(readFileSync(require.resolve('./fixtures/empty_results.xml'), 'utf8')); + const tests = parser.parse(readFileSync(require.resolve('./fixtures/empty_results.xml'), 'utf8')); assert.deepStrictEqual(tests, []); }); it('ignores skipped tests', () => { - const tests = parser(readFileSync(require.resolve('./fixtures/go_skip.xml'), 'utf8')); + const tests = parser.parse(readFileSync(require.resolve('./fixtures/go_skip.xml'), 'utf8')); assert.strictEqual(tests.length, 1); }); }); From caf846800177e61385f13b352c922b218e948f07 Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Thu, 20 Aug 2020 18:54:29 -0400 Subject: [PATCH 11/18] two tests for new post-build endpoint --- packages/api/lib/xunit-parser.js | 66 ++++++++++++++++------------ packages/api/src/post-build.js | 12 +++-- packages/api/test/post-build-test.js | 57 +++++++++++++++++++++++- 3 files changed, 103 insertions(+), 32 deletions(-) diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js index c17fe64f..b3617122 100644 --- a/packages/api/lib/xunit-parser.js +++ b/packages/api/lib/xunit-parser.js @@ -82,33 +82,45 @@ class Parser { return tests; } - // // IMPORTANT: All values that will be used as keys in Firestore must be escaped with the firestoreEncode function - // static cleanXunitBuildInfo (metadata) { - // const timestampNumb = Date.parse(metadata.timestamp); - // const timestamp = isNaN(timestampNumb) ? new Date() : new Date(timestampNumb); - - // const returnVal = { - // repoId: firebaseEncode(decodeURIComponent(metadata.repoId)), - // organization: metadata.organization, - // timestamp, - // url: metadata.url, - // environment: PostBuildHandler.cleanEnvironment(metadata), - // buildId: firebaseEncode(decodeURIComponent(metadata.buildId)), - // sha: metadata.sha, - // name: metadata.name, - // description: metadata.description, - // buildmessage: metadata.buildmessage - // }; - - // // validate data - // for (const prop in returnVal) { - // if (!returnVal[prop]) { - // throw new InvalidParameterError('Missing All Build Meta Data Info - ' + prop); - // } - // } - - // return returnVal; - // } + // IMPORTANT: All values that will be used as keys in Firestore must be escaped with the firestoreEncode function + cleanXunitBuildInfo (metadata) { + return {}; + // const timestampNumb = Date.parse(metadata.timestamp); + // const timestamp = isNaN(timestampNumb) ? new Date() : new Date(timestampNumb); + + // const returnVal = { + // repoId: firebaseEncode(decodeURIComponent(metadata.repoId)), + // organization: metadata.organization, + // timestamp, + // url: metadata.url, + // environment: PostBuildHandler.cleanEnvironment(metadata), + // buildId: firebaseEncode(decodeURIComponent(metadata.buildId)), + // sha: metadata.sha, + // name: metadata.name, + // description: metadata.description, + // buildmessage: metadata.buildmessage + // }; + + // // validate data + // for (const prop in returnVal) { + // if (!returnVal[prop]) { + // throw new InvalidParameterError('Missing All Build Meta Data Info - ' + prop); + // } + // } + + // /** + // environment: + // matrix + // os + // ref (linking to build) + // tag + + // sha: should make correct, should be in Buildcop + + // */ + + // return returnVal; + } } // console.log(parse(readFileSync(require.resolve('../test/fixtures/one_failed.xml'), 'utf8'))); diff --git a/packages/api/src/post-build.js b/packages/api/src/post-build.js index 09bf5cf4..d454fd09 100644 --- a/packages/api/src/post-build.js +++ b/packages/api/src/post-build.js @@ -32,6 +32,10 @@ class PostBuildHandler { this.client = client; } + static async addBuild (testCases, buildInfo, client, collectionName) { + await addBuild(testCases, buildInfo, client, collectionName); + } + static parseEnvironment (metadata) { var envData = { os: metadata.os.os || 'Not specified', @@ -233,7 +237,7 @@ class PostBuildHandler { throw new UnauthorizedError('Flaky does not store tests for private repos'); } - await addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); + await PostBuildHandler.addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); res.send({ message: 'successfully added build' }); } catch (err) { handleError(res, err); @@ -259,7 +263,7 @@ class PostBuildHandler { throw new UnauthorizedError('Must have valid Github Token to post build'); } - await addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); + await PostBuildHandler.addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); res.send({ message: 'successfully added build' }); } catch (err) { handleError(res, err); @@ -270,14 +274,14 @@ class PostBuildHandler { // required keys in the req.body.metadata are the inputs for addBuild in src/add-build.js this.app.post('/api/build/xml', async (req, res, next) => { try { - if (req.body.metadata.token !== process.env.buildCopSecret) { + if (req.headers.authorization !== process.env.PRIVATE_POSTING_TOKEN) { throw new UnauthorizedError('Invalid Secret. Only Google Employees may use this endpoint.'); } const testCases = xunitParser.parse(req.body.data); const buildInfo = xunitParser.cleanXunitBuildInfo(req.body.metadata); - await addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); + await PostBuildHandler.addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); res.send({ message: 'successfully added build' }); } catch (err) { handleError(res, err); diff --git a/packages/api/test/post-build-test.js b/packages/api/test/post-build-test.js index 67621ba0..3d981cf5 100644 --- a/packages/api/test/post-build-test.js +++ b/packages/api/test/post-build-test.js @@ -23,11 +23,12 @@ const EXAMPLE_TAP_MANGLED = fs.readFileSync(path.join(__dirname, 'res/mangledtap const EXAMPLE_TAP_NESTED = fs.readFileSync(path.join(__dirname, 'res/nestedtap.tap'), 'utf8'); const EXAMPLE_STUFF_ON_TOP = fs.readFileSync(path.join(__dirname, 'res/stuffontoptap.tap'), 'utf8'); -const { describe, before, after, it } = require('mocha'); +const { describe, before, after, it, afterEach } = require('mocha'); const { v4: uuidv4 } = require('uuid'); const firebaseEncode = require('../lib/firebase-encode'); const nock = require('nock'); +const sinon = require('sinon'); const validNockResponse = require('./res/sample-validate-resp.json'); const { deleteRepo } = require('../lib/deleter'); @@ -37,6 +38,8 @@ const client = require('../src/firestore.js'); const assert = require('assert'); const fetch = require('node-fetch'); +const xunitParser = require('../lib/xunit-parser'); + nock.disableNetConnect(); nock.enableNetConnect(/^(?!.*github\.com).*$/); // only disable requests on github.com @@ -259,6 +262,58 @@ describe('Posting Builds', () => { assert.strictEqual(result.data().environment.ref, 'master'); }); + describe('xunit endpoint', async () => { + let stubs = []; + + afterEach(() => { + /** Cleanup **/ + stubs.forEach(stubbed => { + stubbed.restore(); + }); + stubs = []; + }); + + it('does not allow a post if no password is included', async () => { + stubs.push(sinon.stub(PostBuildHandler, 'addBuild')); + + const bodyData = fs.readFileSync(require.resolve('./fixtures/passed.xml'), 'utf8'); + process.env.PRIVATE_POSTING_TOKEN = 'hello'; + + const resp = await fetch('http://127.0.0.1:3000/api/build/xml', { + method: 'post', + body: JSON.stringify({ + data: bodyData, + metadata: {} + }), + headers: { 'Content-Type': 'application/json' } + }); + + assert.strictEqual(resp.status, 401); + assert.strictEqual(resp.statusText, 'Unauthorized'); + }); + + it('calls addBuild with appropriate data when authentication token is included', async () => { + stubs.push(sinon.stub(PostBuildHandler, 'addBuild')); + stubs.push(sinon.stub(xunitParser, 'parse').returns([])); + stubs.push(sinon.stub(xunitParser, 'cleanXunitBuildInfo').returns({})); + + const bodyData = fs.readFileSync(require.resolve('./fixtures/passed.xml'), 'utf8'); + process.env.PRIVATE_POSTING_TOKEN = 'hello'; + + const resp = await fetch('http://127.0.0.1:3000/api/build/xml', { + method: 'post', + body: JSON.stringify({ + data: bodyData, + metadata: {} + }), + headers: { 'content-type': 'application/json', Authorization: process.env.PRIVATE_POSTING_TOKEN } + }); + + assert.strictEqual(resp.status, 200); + assert(stubs[0].calledWith([], {})); + }); + }); + after(async () => { var parsedPayload = JSON.parse(EXAMPLE_PAYLOAD); var parsedPayloadRaw = JSON.parse(EXAMPLE_PAYLOAD_RAW); From 1530917fa2d6fa1cff2e2f4caa3326473eabc507 Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Thu, 20 Aug 2020 19:31:42 -0400 Subject: [PATCH 12/18] delete commented out code --- packages/api/lib/xunit-parser.js | 35 -------------------------------- 1 file changed, 35 deletions(-) diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js index b3617122..c3978ef7 100644 --- a/packages/api/lib/xunit-parser.js +++ b/packages/api/lib/xunit-parser.js @@ -85,41 +85,6 @@ class Parser { // IMPORTANT: All values that will be used as keys in Firestore must be escaped with the firestoreEncode function cleanXunitBuildInfo (metadata) { return {}; - // const timestampNumb = Date.parse(metadata.timestamp); - // const timestamp = isNaN(timestampNumb) ? new Date() : new Date(timestampNumb); - - // const returnVal = { - // repoId: firebaseEncode(decodeURIComponent(metadata.repoId)), - // organization: metadata.organization, - // timestamp, - // url: metadata.url, - // environment: PostBuildHandler.cleanEnvironment(metadata), - // buildId: firebaseEncode(decodeURIComponent(metadata.buildId)), - // sha: metadata.sha, - // name: metadata.name, - // description: metadata.description, - // buildmessage: metadata.buildmessage - // }; - - // // validate data - // for (const prop in returnVal) { - // if (!returnVal[prop]) { - // throw new InvalidParameterError('Missing All Build Meta Data Info - ' + prop); - // } - // } - - // /** - // environment: - // matrix - // os - // ref (linking to build) - // tag - - // sha: should make correct, should be in Buildcop - - // */ - - // return returnVal; } } From 694afe6f63f6216c71997476019e41ab5b6a2986 Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Thu, 20 Aug 2020 19:32:36 -0400 Subject: [PATCH 13/18] one last commented out line deleted --- packages/api/lib/xunit-parser.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js index c3978ef7..69048a02 100644 --- a/packages/api/lib/xunit-parser.js +++ b/packages/api/lib/xunit-parser.js @@ -88,6 +88,5 @@ class Parser { } } -// console.log(parse(readFileSync(require.resolve('../test/fixtures/one_failed.xml'), 'utf8'))); const parserHandler = new Parser(); module.exports = parserHandler; From 8c30ded13de6ea302337f27b7c98bdc4299b308c Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Thu, 20 Aug 2020 19:54:13 -0400 Subject: [PATCH 14/18] switch test name parser to use regex --- packages/api/lib/xunit-parser.js | 16 ++++++++-------- packages/api/test/xunit-parser-test.js | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js index 69048a02..f8f641f5 100644 --- a/packages/api/lib/xunit-parser.js +++ b/packages/api/lib/xunit-parser.js @@ -32,17 +32,11 @@ class Parser { testsuites = [testsuites]; } for (const suite of testsuites) { - // Ruby doesn't always have _attributes. + // Ruby doesn't always have _attributes. let testsuiteName = suite._attributes ? suite._attributes.name : undefined; // Get rid of github.com/orgName/repoName/ - testsuiteName = testsuiteName.substring(testsuiteName.indexOf('/') + 1); - testsuiteName = testsuiteName.substring(testsuiteName.indexOf('/') + 1); - const index = testsuiteName.indexOf('/'); - if (index !== -1) { testsuiteName = testsuiteName.substring(index + 1); } else { - // There is nothing past the repo url - testsuiteName = ''; - } + testsuiteName = this.trim(testsuiteName); let testcases = suite.testcase; // If there were no tests in the package, continue. @@ -82,6 +76,12 @@ class Parser { return tests; } + trim (url) { + const updated = url.replace(/(.)*github.com\/[a-zA-Z-]*\/[a-zA-Z-]*/g, ''); + if (updated.length > 1) return updated.substring(1); // Get rid of starting `/` + return updated; + } + // IMPORTANT: All values that will be used as keys in Firestore must be escaped with the firestoreEncode function cleanXunitBuildInfo (metadata) { return {}; diff --git a/packages/api/test/xunit-parser-test.js b/packages/api/test/xunit-parser-test.js index 1b7cc3a2..5f63bd7e 100644 --- a/packages/api/test/xunit-parser-test.js +++ b/packages/api/test/xunit-parser-test.js @@ -17,7 +17,7 @@ const assert = require('assert'); const parser = require('../lib/xunit-parser'); const { readFileSync } = require('fs'); -describe('xunit-parser-test', () => { +describe.only('xunit-parser-test', () => { it('stores the correct number of tests', () => { const tests = parser.parse(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); assert.strictEqual(tests.length, 4); From def620cc8eef913159c8ae3b083f7fe7bd5cc3ba Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Thu, 20 Aug 2020 20:43:32 -0400 Subject: [PATCH 15/18] split line into shorter ones --- packages/api/lib/xunit-parser.js | 9 ++++++++- packages/api/test/xunit-parser-test.js | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js index f8f641f5..da20dda3 100644 --- a/packages/api/lib/xunit-parser.js +++ b/packages/api/lib/xunit-parser.js @@ -57,7 +57,14 @@ class Parser { const failure = testcase.failure; const error = testcase.error; - const testCaseRun = new TestCaseRun((failure === undefined && error === undefined) ? 'ok' : 'not ok', (testsuiteName !== '') ? testsuiteName + '/' + testcase._attributes.name : testcase._attributes.name); + const okayMessage = (failure === undefined && error === undefined) ? 'ok' : 'not ok'; + let name = testcase._attributes.name; + + if (testsuiteName.length > 0) { + name = testsuiteName + '/' + name; + } + + const testCaseRun = new TestCaseRun(okayMessage, name); if (!testCaseRun.successful) { // Here we must have a failure or an error. diff --git a/packages/api/test/xunit-parser-test.js b/packages/api/test/xunit-parser-test.js index 5f63bd7e..1b7cc3a2 100644 --- a/packages/api/test/xunit-parser-test.js +++ b/packages/api/test/xunit-parser-test.js @@ -17,7 +17,7 @@ const assert = require('assert'); const parser = require('../lib/xunit-parser'); const { readFileSync } = require('fs'); -describe.only('xunit-parser-test', () => { +describe('xunit-parser-test', () => { it('stores the correct number of tests', () => { const tests = parser.parse(readFileSync(require.resolve('./fixtures/one_failed.xml'), 'utf8')); assert.strictEqual(tests.length, 4); From 469d2547202a1cfdb51caca372654061e16cbf36 Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Thu, 20 Aug 2020 20:51:28 -0400 Subject: [PATCH 16/18] add comments --- packages/api/lib/xunit-parser.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js index da20dda3..5c6d5cbd 100644 --- a/packages/api/lib/xunit-parser.js +++ b/packages/api/lib/xunit-parser.js @@ -16,6 +16,12 @@ const xmljs = require('xml-js'); const TestCaseRun = require('../lib/testrun'); class Parser { + /** + Parse the xunit test results, converting them to an array of testrun objects. + Each of these testrun objects contains whether the test passed, its name, and + its failure message (if failed). + @param xmlString - The complete xunit string of test results sent to the endpoint + */ parse (xmlString) { const obj = xmljs.xml2js(xmlString, { compact: true }); const tests = []; @@ -83,6 +89,11 @@ class Parser { return tests; } + /** + The url contains 'github.com/:org/:repo', optionally followed by a path. + This method trims the url down to only the string following `repo`, which may be empty. + @param url - the Github url of where a test came from + */ trim (url) { const updated = url.replace(/(.)*github.com\/[a-zA-Z-]*\/[a-zA-Z-]*/g, ''); if (updated.length > 1) return updated.substring(1); // Get rid of starting `/` @@ -90,6 +101,10 @@ class Parser { } // IMPORTANT: All values that will be used as keys in Firestore must be escaped with the firestoreEncode function + /** + Parse the build data and convert it into a JSON format that can easily be read and stored. + @param metadata - contains all data sent to the endpoint besides the actual test results. + */ cleanXunitBuildInfo (metadata) { return {}; } From 9203c6e6e10c51ed9ef6b8607b00a5b4a7d096a7 Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Thu, 20 Aug 2020 21:07:18 -0400 Subject: [PATCH 17/18] replace stubs array with sinon.sandbox --- packages/api/lib/xunit-parser.js | 2 +- packages/api/test/post-build-test.js | 20 +++++++++----------- packages/api/test/server-test.js | 28 +++++++++++++--------------- 3 files changed, 23 insertions(+), 27 deletions(-) diff --git a/packages/api/lib/xunit-parser.js b/packages/api/lib/xunit-parser.js index 5c6d5cbd..f2fb8d19 100644 --- a/packages/api/lib/xunit-parser.js +++ b/packages/api/lib/xunit-parser.js @@ -17,7 +17,7 @@ const TestCaseRun = require('../lib/testrun'); class Parser { /** - Parse the xunit test results, converting them to an array of testrun objects. + Parse the xunit test results, converting them to an array of testrun objects. Each of these testrun objects contains whether the test passed, its name, and its failure message (if failed). @param xmlString - The complete xunit string of test results sent to the endpoint diff --git a/packages/api/test/post-build-test.js b/packages/api/test/post-build-test.js index 3d981cf5..3376d9bf 100644 --- a/packages/api/test/post-build-test.js +++ b/packages/api/test/post-build-test.js @@ -263,18 +263,14 @@ describe('Posting Builds', () => { }); describe('xunit endpoint', async () => { - let stubs = []; + const sandbox = sinon.createSandbox(); afterEach(() => { - /** Cleanup **/ - stubs.forEach(stubbed => { - stubbed.restore(); - }); - stubs = []; + sandbox.restore(); }); it('does not allow a post if no password is included', async () => { - stubs.push(sinon.stub(PostBuildHandler, 'addBuild')); + sandbox.stub(PostBuildHandler, 'addBuild'); const bodyData = fs.readFileSync(require.resolve('./fixtures/passed.xml'), 'utf8'); process.env.PRIVATE_POSTING_TOKEN = 'hello'; @@ -293,9 +289,9 @@ describe('Posting Builds', () => { }); it('calls addBuild with appropriate data when authentication token is included', async () => { - stubs.push(sinon.stub(PostBuildHandler, 'addBuild')); - stubs.push(sinon.stub(xunitParser, 'parse').returns([])); - stubs.push(sinon.stub(xunitParser, 'cleanXunitBuildInfo').returns({})); + const addBuildStub = sinon.stub(PostBuildHandler, 'addBuild'); + sandbox.stub(xunitParser, 'parse').returns([]); + sandbox.stub(xunitParser, 'cleanXunitBuildInfo').returns({}); const bodyData = fs.readFileSync(require.resolve('./fixtures/passed.xml'), 'utf8'); process.env.PRIVATE_POSTING_TOKEN = 'hello'; @@ -310,7 +306,9 @@ describe('Posting Builds', () => { }); assert.strictEqual(resp.status, 200); - assert(stubs[0].calledWith([], {})); + assert(addBuildStub.calledWith([], {})); + + addBuildStub.restore(); }); }); diff --git a/packages/api/test/server-test.js b/packages/api/test/server-test.js index e2156304..8f7e8a4e 100644 --- a/packages/api/test/server-test.js +++ b/packages/api/test/server-test.js @@ -24,15 +24,11 @@ const auth = require('../src/auth.js'); const repo = require('../src/repository.js'); describe('flaky express server', () => { - let stubs = []; + const sandbox = sinon.createSandbox(); const frontendUrl = 'https://flaky-dashboard.web.app/home'; afterEach(() => { - /** Cleanup **/ - stubs.forEach(stubbed => { - stubbed.restore(); - }); - stubs = []; + sandbox.restore(); }); it('should have the mocked environment variables', () => { @@ -41,7 +37,7 @@ describe('flaky express server', () => { describe('get /repo to delete a test', async () => { it('generates a GitHub redirect', async () => { - stubs.push(sinon.stub(repo, 'storeTicket').returns(true)); + sandbox.stub(repo, 'storeTicket').returns(true); const resp = await fetch('http://0.0.0.0:3000/api/repo/my-org/my-repo/test/deleteurl?testname=my-test&redirect=' + process.env.FRONTEND_URL, { method: 'GET' }); @@ -51,7 +47,6 @@ describe('flaky express server', () => { it('stores correct information in the ticket', async () => { const stubbed = sinon.stub(repo, 'storeTicket'); - stubs.push(stubbed); await fetch('http://0.0.0.0:3000/api/repo/my-org/my-repo/test/deleteurl?testname=my-test&redirect=' + process.env.FRONTEND_URL, { method: 'GET' @@ -64,12 +59,14 @@ describe('flaky express server', () => { testName: 'my-test', redirect: process.env.FRONTEND_URL })); + + stubbed.restore(); }); }); describe('get /repo to delete a repository', async () => { it('generates a GitHub redirect', async () => { - stubs.push(sinon.stub(repo, 'storeTicket').returns(true)); + sandbox.stub(repo, 'storeTicket').returns(true); const resp = await fetch('http://0.0.0.0:3000/api/repo/my-org/my-repo/deleteurl?redirect=' + process.env.FRONTEND_URL, { method: 'GET' }); @@ -79,7 +76,6 @@ describe('flaky express server', () => { it('stores correct information in the ticket', async () => { const stubbed = sinon.stub(repo, 'storeTicket'); - stubs.push(stubbed); await fetch('http://0.0.0.0:3000/api/repo/my-org/my-repo/deleteurl?redirect=' + process.env.FRONTEND_URL, { method: 'GET' @@ -91,6 +87,8 @@ describe('flaky express server', () => { repoId: 'my-repo', redirect: process.env.FRONTEND_URL })); + + stubbed.restore(); }); }); @@ -99,17 +97,17 @@ describe('flaky express server', () => { /** Stubbing **/ const queryObject = querystring.stringify({ access_token: 'fake-access-token' }); - stubs.push(sinon.stub(auth, 'retrieveAccessToken').returns(queryObject)); + sandbox.stub(auth, 'retrieveAccessToken').returns(queryObjectstu); - stubs.push(sinon.stub(auth, 'retrieveUserPermission').returns('write')); + sandbox.stub(auth, 'retrieveUserPermission').returns('write'); - stubs.push(sinon.stub(repo, 'performTicketIfAllowed').returns(true)); + sandbox.stub(repo, 'performTicketIfAllowed').returns(true); const fakeState = 'testing-state'; - stubs.push(sinon.stub(repo, 'getTicket').returns({ + sandbox.stub(repo, 'getTicket').returns({ state: fakeState, redirect: process.env.FRONTEND_URL - })); + }); /** Testing **/ const resp = await fetch('http://0.0.0.0:3000/api/callback?state=' + fakeState + '&code=ANYTHING', { From d952a42b2ed0844767035adff0bd0b3351938ba1 Mon Sep 17 00:00:00 2001 From: Cedonia Peterson Date: Thu, 20 Aug 2020 21:27:31 -0400 Subject: [PATCH 18/18] refactor add-build so that it can more easily be tested --- packages/api/src/add-build.js | 2 +- packages/api/src/post-build.js | 12 ++++-------- packages/api/test/addbuild-getbuild.js | 2 +- packages/api/test/analytics-test.js | 2 +- packages/api/test/get-batches.js | 2 +- packages/api/test/post-build-test.js | 5 +++-- packages/api/test/server-test.js | 2 +- 7 files changed, 12 insertions(+), 15 deletions(-) diff --git a/packages/api/src/add-build.js b/packages/api/src/add-build.js index 6b45684f..20ef4d39 100644 --- a/packages/api/src/add-build.js +++ b/packages/api/src/add-build.js @@ -278,4 +278,4 @@ async function updateRepoDoc (buildInfo, computedData, mostRecent, dbRepo) { // db read operation to fix this. This would only be visible when ordering by priority on org page. } -module.exports = addBuild; +module.exports = { addBuild }; diff --git a/packages/api/src/post-build.js b/packages/api/src/post-build.js index d454fd09..aec96a62 100644 --- a/packages/api/src/post-build.js +++ b/packages/api/src/post-build.js @@ -16,7 +16,7 @@ // NOTE: relies on global.headCollection to be the high level repository -const addBuild = require('../src/add-build'); +const AddBuildHandler = require('../src/add-build'); const TestCaseRun = require('../lib/testrun'); const TapParser = require('tap-parser'); const xunitParser = require('../lib/xunit-parser'); @@ -32,10 +32,6 @@ class PostBuildHandler { this.client = client; } - static async addBuild (testCases, buildInfo, client, collectionName) { - await addBuild(testCases, buildInfo, client, collectionName); - } - static parseEnvironment (metadata) { var envData = { os: metadata.os.os || 'Not specified', @@ -237,7 +233,7 @@ class PostBuildHandler { throw new UnauthorizedError('Flaky does not store tests for private repos'); } - await PostBuildHandler.addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); + await AddBuildHandler.addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); res.send({ message: 'successfully added build' }); } catch (err) { handleError(res, err); @@ -263,7 +259,7 @@ class PostBuildHandler { throw new UnauthorizedError('Must have valid Github Token to post build'); } - await PostBuildHandler.addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); + await AddBuildHandler.addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); res.send({ message: 'successfully added build' }); } catch (err) { handleError(res, err); @@ -281,7 +277,7 @@ class PostBuildHandler { const testCases = xunitParser.parse(req.body.data); const buildInfo = xunitParser.cleanXunitBuildInfo(req.body.metadata); - await PostBuildHandler.addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); + await AddBuildHandler.addBuild(PostBuildHandler.removeDuplicateTestCases(testCases), buildInfo, this.client, global.headCollection); res.send({ message: 'successfully added build' }); } catch (err) { handleError(res, err); diff --git a/packages/api/test/addbuild-getbuild.js b/packages/api/test/addbuild-getbuild.js index c91706f0..adb239b5 100644 --- a/packages/api/test/addbuild-getbuild.js +++ b/packages/api/test/addbuild-getbuild.js @@ -19,7 +19,7 @@ const client = require('../src/firestore.js'); const firebaseEncode = require('../lib/firebase-encode'); const TestCaseRun = require('../lib/testrun'); -const addBuild = require('../src/add-build'); +const { addBuild } = require('../src/add-build'); const { deleteTest, deleteRepo } = require('../lib/deleter'); const fetch = require('node-fetch'); diff --git a/packages/api/test/analytics-test.js b/packages/api/test/analytics-test.js index 29c07a84..eb69ff04 100644 --- a/packages/api/test/analytics-test.js +++ b/packages/api/test/analytics-test.js @@ -18,7 +18,7 @@ const { v4: uuidv4 } = require('uuid'); const client = require('../src/firestore.js'); const TestCaseRun = require('../lib/testrun'); -const addBuild = require('../src/add-build'); +const { addBuild } = require('../src/add-build'); const { deleteRepo } = require('../lib/deleter'); const buildInfo = { diff --git a/packages/api/test/get-batches.js b/packages/api/test/get-batches.js index 18f877a1..e4043a89 100644 --- a/packages/api/test/get-batches.js +++ b/packages/api/test/get-batches.js @@ -20,7 +20,7 @@ const sinon = require('sinon'); const assert = require('assert'); const fetch = require('node-fetch'); const firebaseEncode = require('../lib/firebase-encode'); -const addBuild = require('../src/add-build'); +const { addBuild } = require('../src/add-build'); const { deleteRepo } = require('../lib/deleter'); const client = require('../src/firestore'); diff --git a/packages/api/test/post-build-test.js b/packages/api/test/post-build-test.js index 3376d9bf..179a41aa 100644 --- a/packages/api/test/post-build-test.js +++ b/packages/api/test/post-build-test.js @@ -33,6 +33,7 @@ const validNockResponse = require('./res/sample-validate-resp.json'); const { deleteRepo } = require('../lib/deleter'); const PostBuildHandler = require('../src/post-build.js'); +const AddBuildHandler = require('../src/add-build'); const client = require('../src/firestore.js'); const assert = require('assert'); @@ -270,7 +271,7 @@ describe('Posting Builds', () => { }); it('does not allow a post if no password is included', async () => { - sandbox.stub(PostBuildHandler, 'addBuild'); + sandbox.stub(AddBuildHandler, 'addBuild'); const bodyData = fs.readFileSync(require.resolve('./fixtures/passed.xml'), 'utf8'); process.env.PRIVATE_POSTING_TOKEN = 'hello'; @@ -289,7 +290,7 @@ describe('Posting Builds', () => { }); it('calls addBuild with appropriate data when authentication token is included', async () => { - const addBuildStub = sinon.stub(PostBuildHandler, 'addBuild'); + const addBuildStub = sinon.stub(AddBuildHandler, 'addBuild'); sandbox.stub(xunitParser, 'parse').returns([]); sandbox.stub(xunitParser, 'cleanXunitBuildInfo').returns({}); diff --git a/packages/api/test/server-test.js b/packages/api/test/server-test.js index 8f7e8a4e..6731b8b7 100644 --- a/packages/api/test/server-test.js +++ b/packages/api/test/server-test.js @@ -97,7 +97,7 @@ describe('flaky express server', () => { /** Stubbing **/ const queryObject = querystring.stringify({ access_token: 'fake-access-token' }); - sandbox.stub(auth, 'retrieveAccessToken').returns(queryObjectstu); + sandbox.stub(auth, 'retrieveAccessToken').returns(queryObject); sandbox.stub(auth, 'retrieveUserPermission').returns('write');