From 2e4151d50e685787498a41f98cc22ac7adb39674 Mon Sep 17 00:00:00 2001 From: Robert Austin Date: Mon, 24 Jul 2023 10:50:13 -0400 Subject: [PATCH] Transform Security's Cypress Junit reports to have the same format as those produced by FTR and Jest (#162279) ## Summary Cypress produced Junit reports, but the failed-test-reporter and various github and kibanamachine workflows rely on a specifically formatted junit report that includes an encoded version of the spec file path in each testcase. For FTR and Jest, these specially formatted junit reports are created by a custom reporter. Due to the architecture of Cypress, re-using those would be difficult. Instead this PR adds a script that reads, transforms, and updates all the junit reports created by Cypress. ### TODO Some work is not covered in this PR. I need to merge this change to test that flaky test triaging works in buildkite and kibana machine (note: if you know how to validate this without merging it, please reach out!) After I'm confident that this works, I'll open follow up PRs to do the following: ```[tasklist] ### Follow up work - [ ] Enable this script for test_serverless cypress tests - [ ] Enable this script for threat intelligence cypress tests (optional) - [ ] Enable this script for fleet (optional) ``` ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../security_solution/jest.config.dev.js | 1 + x-pack/plugins/security_solution/package.json | 5 +- .../security_solution/scripts/jest.config.js | 16 ++ .../scripts/junit_transformer/README.md | 9 + .../junit_transformer.test.ts.snap | 21 ++ .../fixtures/suite_with_failing_test.xml | 17 ++ .../scripts/junit_transformer/index.js | 9 + .../junit_transformer.test.ts | 61 +++++ .../junit_transformer/junit_transformer.ts | 30 +++ .../scripts/junit_transformer/lib.ts | 254 ++++++++++++++++++ 10 files changed, 421 insertions(+), 2 deletions(-) create mode 100644 x-pack/plugins/security_solution/scripts/jest.config.js create mode 100644 x-pack/plugins/security_solution/scripts/junit_transformer/README.md create mode 100644 x-pack/plugins/security_solution/scripts/junit_transformer/__snapshots__/junit_transformer.test.ts.snap create mode 100644 x-pack/plugins/security_solution/scripts/junit_transformer/fixtures/suite_with_failing_test.xml create mode 100644 x-pack/plugins/security_solution/scripts/junit_transformer/index.js create mode 100644 x-pack/plugins/security_solution/scripts/junit_transformer/junit_transformer.test.ts create mode 100644 x-pack/plugins/security_solution/scripts/junit_transformer/junit_transformer.ts create mode 100644 x-pack/plugins/security_solution/scripts/junit_transformer/lib.ts diff --git a/x-pack/plugins/security_solution/jest.config.dev.js b/x-pack/plugins/security_solution/jest.config.dev.js index 1810adc42e362..1aaace56c5f88 100644 --- a/x-pack/plugins/security_solution/jest.config.dev.js +++ b/x-pack/plugins/security_solution/jest.config.dev.js @@ -12,5 +12,6 @@ module.exports = { '/x-pack/plugins/security_solution/common/*/jest.config.js', '/x-pack/plugins/security_solution/server/*/jest.config.js', '/x-pack/plugins/security_solution/public/*/jest.config.js', + '/x-pack/plugins/security_solution/scripts/junit_transformer/*/jest.config.js', ], }; diff --git a/x-pack/plugins/security_solution/package.json b/x-pack/plugins/security_solution/package.json index 01419bb09e470..a564416154176 100644 --- a/x-pack/plugins/security_solution/package.json +++ b/x-pack/plugins/security_solution/package.json @@ -25,9 +25,10 @@ "cypress:dw:endpoint:open": "node ./scripts/start_cypress_parallel open --config-file ./public/management/cypress_endpoint.config.ts ts --ftr-config-file ../../../../../../x-pack/test/defend_workflows_cypress/endpoint_config", "cypress:investigations:run": "yarn cypress:run:reporter --browser chrome --spec './cypress/e2e/investigations/**/*.cy.ts' --ftr-config-file ../../../../../../x-pack/test/security_solution_cypress/cli_config; status=$?; yarn junit:merge && exit $status", "cypress:explore:run": "yarn cypress:run:reporter --browser chrome --spec './cypress/e2e/explore/**/*.cy.ts' --ftr-config-file ../../../../../../x-pack/test/security_solution_cypress/cli_config; status=$?; yarn junit:merge && exit $status", - "junit:merge": "../../../node_modules/.bin/mochawesome-merge ../../../target/kibana-security-solution/cypress/results/mochawesome*.json > ../../../target/kibana-security-solution/cypress/results/output.json && ../../../node_modules/.bin/marge ../../../target/kibana-security-solution/cypress/results/output.json --reportDir ../../../target/kibana-security-solution/cypress/results && mkdir -p ../../../target/junit && cp ../../../target/kibana-security-solution/cypress/results/*.xml ../../../target/junit/", + "junit:merge": "../../../node_modules/.bin/mochawesome-merge ../../../target/kibana-security-solution/cypress/results/mochawesome*.json > ../../../target/kibana-security-solution/cypress/results/output.json && ../../../node_modules/.bin/marge ../../../target/kibana-security-solution/cypress/results/output.json --reportDir ../../../target/kibana-security-solution/cypress/results && yarn junit:transform && mkdir -p ../../../target/junit && cp ../../../target/kibana-security-solution/cypress/results/*.xml ../../../target/junit/", "test:generate": "node scripts/endpoint/resolver_generator", "mappings:generate": "node scripts/mappings/mappings_generator", - "mappings:load": "node scripts/mappings/mappings_loader" + "mappings:load": "node scripts/mappings/mappings_loader", + "junit:transform": "node scripts/junit_transformer --pathPattern '../../../target/kibana-security-solution/cypress/results/*.xml' --rootDirectory ../../../ --reportName 'Security Solution Cypress' --writeInPlace" } } diff --git a/x-pack/plugins/security_solution/scripts/jest.config.js b/x-pack/plugins/security_solution/scripts/jest.config.js new file mode 100644 index 0000000000000..dba0d6bc5bade --- /dev/null +++ b/x-pack/plugins/security_solution/scripts/jest.config.js @@ -0,0 +1,16 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +module.exports = { + preset: '@kbn/test', + rootDir: '../../../..', + roots: ['/x-pack/plugins/security_solution/scripts'], + coverageDirectory: + '/target/kibana-coverage/jest/x-pack/plugins/security_solution/scripts', + coverageReporters: ['text', 'html'], + collectCoverageFrom: ['/x-pack/plugins/security_solution/scripts/**/*.{ts,tsx}'], +}; diff --git a/x-pack/plugins/security_solution/scripts/junit_transformer/README.md b/x-pack/plugins/security_solution/scripts/junit_transformer/README.md new file mode 100644 index 0000000000000..6e8a834da756b --- /dev/null +++ b/x-pack/plugins/security_solution/scripts/junit_transformer/README.md @@ -0,0 +1,9 @@ +The failed test reporter creates github issues based on junit reports. Github workflows, and kibanamachine workflows, allow the Kibana Operations team to track and triage flaky tests. These workflows rely on those github issues, specifically their titles, to work. The titles of the github issues contain an encoded version of the file path that contains the failing test. + +This process is facilitated by custom mocha/junit reporters written for the functional test runner and jest. These reporters encode the file name of each spec file and include it in an attribute on elements in the junit report. + +There is no such custom mocha reporter for Cypress, and due to the architecture of Cypress, reusing the existing custom mocha reports, or any of their existing code, is not feasible. Cypress runs in its own process, with its own version of node, and that environment is incompatible with running babel-register. This means we cannot easily interpret the code that implements the existing custom mocha reporters from within Cypress. + +We could compile a library using the code from those custom junit reporters, but there is no established pattern or tooling for doing that. + +For there reasons, our approach is to transform the junit report created by Cypress into a format consumable by the failed test reporter and the kibana operations triage scripts. This script does that. diff --git a/x-pack/plugins/security_solution/scripts/junit_transformer/__snapshots__/junit_transformer.test.ts.snap b/x-pack/plugins/security_solution/scripts/junit_transformer/__snapshots__/junit_transformer.test.ts.snap new file mode 100644 index 0000000000000..1474bf9e60c77 --- /dev/null +++ b/x-pack/plugins/security_solution/scripts/junit_transformer/__snapshots__/junit_transformer.test.ts.snap @@ -0,0 +1,21 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`junit_transformer updates the file in place, applying the expected transformation 1`] = ` +" + + + + + + + + AssertionError: Timed out retrying after 150000ms: expected 'http://localhost:5647/app/security/rules/create' to include 'app/security/rules/create1' + at Context.eval (webpack:///./e2e/urls/compatibility.cy.ts:65:13) + + + + + + +" +`; diff --git a/x-pack/plugins/security_solution/scripts/junit_transformer/fixtures/suite_with_failing_test.xml b/x-pack/plugins/security_solution/scripts/junit_transformer/fixtures/suite_with_failing_test.xml new file mode 100644 index 0000000000000..9c803eb829eed --- /dev/null +++ b/x-pack/plugins/security_solution/scripts/junit_transformer/fixtures/suite_with_failing_test.xml @@ -0,0 +1,17 @@ + + + + + + + + + AssertionError: Timed out retrying after 150000ms: expected 'http://localhost:5647/app/security/rules/create' to include 'app/security/rules/create1' + at Context.eval (webpack:///./e2e/urls/compatibility.cy.ts:65:13) + + + + + + + \ No newline at end of file diff --git a/x-pack/plugins/security_solution/scripts/junit_transformer/index.js b/x-pack/plugins/security_solution/scripts/junit_transformer/index.js new file mode 100644 index 0000000000000..f1b0280c4ede8 --- /dev/null +++ b/x-pack/plugins/security_solution/scripts/junit_transformer/index.js @@ -0,0 +1,9 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +require('../../../../../src/setup_node_env'); +require('./junit_transformer'); diff --git a/x-pack/plugins/security_solution/scripts/junit_transformer/junit_transformer.test.ts b/x-pack/plugins/security_solution/scripts/junit_transformer/junit_transformer.test.ts new file mode 100644 index 0000000000000..e25a349b394ac --- /dev/null +++ b/x-pack/plugins/security_solution/scripts/junit_transformer/junit_transformer.test.ts @@ -0,0 +1,61 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +import { promises as fs } from 'fs'; +import { mkdtemp } from 'fs/promises'; +import { tmpdir } from 'os'; +import { join } from 'path'; +import type { CommandArgs } from './lib'; +import { command } from './lib'; + +describe('junit_transformer', () => { + const junitFileName = 'junit.xml'; + let pathPattern: string; + let path: string; + let mockCommandArgs: CommandArgs; + + beforeEach(async () => { + // get a temporary directory + const directory = await mkdtemp(join(tmpdir(), 'junit-transformer-test-')); + + // define a glob pattern that will match the fixture + pathPattern = `${directory}/*`; + + // determine the path for the fixture + path = join(directory, junitFileName); + + // read the fixture and write it to the temporary file + await fs.writeFile( + path, + await fs.readFile(join(__dirname, './fixtures/suite_with_failing_test.xml'), { + encoding: 'utf8', + }) + ); + + mockCommandArgs = { + // define the flags that will be passed to the command + flags: { + pathPattern, + // use the directory as the root directory. This lets us test the relative file path functionality without having a tree of temp files. + rootDirectory: directory, + reportName: 'Test', + writeInPlace: true, + }, + + log: { + info: jest.fn(), + write: jest.fn(), + error: jest.fn(), + success: jest.fn(), + warning: jest.fn(), + }, + }; + }); + it('updates the file in place, applying the expected transformation', async () => { + await command(mockCommandArgs); + expect(await fs.readFile(path, { encoding: 'utf8' })).toMatchSnapshot(); + }); +}); diff --git a/x-pack/plugins/security_solution/scripts/junit_transformer/junit_transformer.ts b/x-pack/plugins/security_solution/scripts/junit_transformer/junit_transformer.ts new file mode 100644 index 0000000000000..1eb29ce1b7ddd --- /dev/null +++ b/x-pack/plugins/security_solution/scripts/junit_transformer/junit_transformer.ts @@ -0,0 +1,30 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { run } from '@kbn/dev-cli-runner'; +import { command } from './lib'; + +/** + * This script processes all junit reports matching a glob pattern. It reads each report, parses it into json, validates that it is a report from Cypress, then transforms the report to a form that can be processed by Kibana Operations workflows and the failed-test-reporter, it then optionally writes the report back, in xml format, to the original file path. + */ +run(command, { + description: ` + Transform junit reports to match the style required by the Kibana Operations flaky test triage workflows such as '/skip'. + `, + flags: { + string: ['pathPattern', 'rootDirectory', 'reportName'], + boolean: ['writeInPlace'], + help: ` + --pathPattern Required, glob passed to globby to select files to operate on + --rootDirectory Required, path of the kibana repo. Used to calcuate the file path of each spec file relative to the Kibana repo + --reportName Required, used as a prefix for the classname. Eventually shows up in the title of flaky test Github issues + --writeInPlace Defaults to false. If passed, rewrite the file in place with transformations. If false, the script will pass the transformed XML as a string to stdout + + If an error is encountered when processing one file, the script will still attempt to process other files. + `, + }, +}); diff --git a/x-pack/plugins/security_solution/scripts/junit_transformer/lib.ts b/x-pack/plugins/security_solution/scripts/junit_transformer/lib.ts new file mode 100644 index 0000000000000..d9033c75d485b --- /dev/null +++ b/x-pack/plugins/security_solution/scripts/junit_transformer/lib.ts @@ -0,0 +1,254 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +/* eslint-disable no-continue */ + +import { createFlagError } from '@kbn/dev-cli-errors'; +import type { RunContext } from '@kbn/dev-cli-runner'; +import { Builder, parseStringPromise } from 'xml2js'; +import { promises as fs } from 'fs'; +import { relative } from 'path'; +import * as t from 'io-ts'; +import { isLeft } from 'fp-ts/lib/Either'; +import { PathReporter } from 'io-ts/lib/PathReporter'; +import globby from 'globby'; + +/** + * Updates the `name` and `classname` attributes of each testcase. + * `name` will have the value of `classname` appended to it. This makes sense because they each contain part of the bdd spec. + * `classname` is replaced with the file path, relative to the kibana project directory, and encoded (by replacing periods with a non-ascii character.) This is the format expected by the failed test reporter and the Kibana Operations flaky test triage workflows. + */ +async function transformedReport({ + reportJson, + specFilePath, + rootDirectory, + reportName, +}: { + reportJson: CypressJunitReport; + specFilePath: string; + rootDirectory: string; + reportName: string; +}): Promise { + for (const testsuite of reportJson.testsuites.testsuite) { + if (!testsuite.testcase) { + // If there are no testcases for this testsuite, skip it + continue; + } + for (const testcase of testsuite.testcase) { + // append the `classname` attribute to the `name` attribute + testcase.$.name = `${testcase.$.name} ${testcase.$.classname}`; + + // calculate the path of the spec file relative to the kibana project directory + const projectRelativePath = relative(rootDirectory, specFilePath); + + // encode the path by relacing dots with a non-ascii character + const encodedPath = projectRelativePath.replace(/\./g, '·'); + + // prepend the encoded path with a report name. This is for display purposes and shows up in the github issue. It is required. Store the value in the `classname` attribute. + testcase.$.classname = `${reportName}.${encodedPath}`; + } + } + + const builder = new Builder(); + // Return the report in an xml string + return builder.buildObject(reportJson); +} + +/** + * Test cases have a name, which is populated with part of the BDD test name, and classname, which is also populated with part of the BDD test name. + */ +const CypressJunitTestCase = t.type({ + $: t.type({ + name: t.string, + classname: t.string, + }), +}); + +/** + * Standard testsuites contain testcase elements, each representing a specific test execution. + */ +const CypressJunitTestSuite = t.intersection([ + t.partial({ + testcase: t.array(CypressJunitTestCase), + }), + t.type({ + $: t.intersection([ + t.type({ + name: t.string, + }), + /* `file` is only found on some suites, namely the 'Root Suite' */ + t.partial({ + file: t.string, + }), + ]), + }), +]); + +const CypressJunitReport = t.type({ + testsuites: t.type({ + testsuite: t.array(CypressJunitTestSuite), + }), +}); + +/** + * This type represents the Cypress-specific flavor of junit report. + **/ +type CypressJunitReport = t.TypeOf; + +/** + * Encapsulate either a successful result, or a recoverable error. This module only throws unrecoverable errors. + */ +type Result = { result: T } | { error: string }; + +/* + * This checks if the junit report contains '·' characters in the classname. This character is used by the kibana operations triage scripts, and the failed test reporter, to replace `.` characters in a path as part of its encoding scheme. If this character is found, we assume that the encoding has already taken place. + */ +function isReportAlreadyProcessed( + report: CypressJunitReport +): { processed: boolean; hadTestCases: true } | { processed: undefined; hadTestCases: false } { + for (const testsuite of report.testsuites.testsuite) { + if (!testsuite.testcase) { + // If there are no testcases for this testsuite, skip it + continue; + } + for (const testcase of testsuite.testcase) { + if (testcase.$.classname.indexOf('·') !== -1) { + return { processed: true, hadTestCases: true }; + } else { + return { processed: false, hadTestCases: true }; + } + } + } + return { processed: undefined, hadTestCases: false }; +} + +/** + * Validate the JSON representation of the Junit XML. + * If there are no errors, this returns `{ result: 'successs' }`, otherwise it returns an error, wrapped in `{ error: string }`. + * + */ +function validatedCypressJunitReport(parsedReport: unknown): Result { + const decoded = CypressJunitReport.decode(parsedReport); + + if (isLeft(decoded)) { + return { + error: `Could not validate data: ${PathReporter.report(decoded).join('\n')}. +`, + }; + } + return { result: decoded.right }; +} + +/** + * Iterate over the test suites and find the root suite, which Cypress populates with the path to the spec file. Return the path. + */ +function findSpecFilePathFromRootSuite(reportJson: CypressJunitReport): Result { + for (const testsuite of reportJson.testsuites.testsuite) { + if (testsuite.$.name === 'Root Suite' && testsuite.$.file) { + return { result: testsuite.$.file }; + } + } + return { + error: "No Root Suite containing a 'file' attribute was found.", + }; +} + +/** + * The CLI command, exported for the sake of automated tests. + */ +export async function command({ flags, log }: CommandArgs) { + if (typeof flags.pathPattern !== 'string' || flags.pathPattern.length === 0) { + throw createFlagError('please provide a single --pathPattern flag'); + } + + if (typeof flags.rootDirectory !== 'string' || flags.rootDirectory.length === 0) { + throw createFlagError('please provide a single --rootDirectory flag'); + } + + if (typeof flags.reportName !== 'string' || flags.reportName.length === 0) { + throw createFlagError('please provide a single --reportName flag'); + } + + for (const path of await globby(flags.pathPattern)) { + // Read the file + const source: string = await fs.readFile(path, 'utf8'); + + // Parse it from XML to json + const unvalidatedReportJson: unknown = await parseStringPromise(source); + + // Apply validation and return the validated report, or an error message + const maybeValidationResult: Result = + validatedCypressJunitReport(unvalidatedReportJson); + + const boilerplate = `This script validates each Junit report to ensure that it was produced by Cypress and that it has not already been processed by this script +This script relies on various assumptions. If your junit report is valid, then you must enhance this script in order to have support for it. If you are not trying to transform a Cypress junit report into a report that is compatible with Kibana Operations workflows, then you are running this script in error.`; + + const logError = (error: string) => { + log.error(`Error while validating ${path}: ${error} +${boilerplate} +`); + }; + + if ('error' in maybeValidationResult) { + logError(maybeValidationResult.error); + // If there is an error, continue trying to process other files. + continue; + } + + const reportJson: CypressJunitReport = maybeValidationResult.result; + + const { processed, hadTestCases } = isReportAlreadyProcessed(reportJson); + if (hadTestCases === false) { + log.warning(`${path} had no test cases. Skipping it. +${boilerplate} +`); + // If there is an error, continue trying to process other files. + continue; + } + + if (processed) { + logError( + "This report appears to have already been transformed because a '·' character was found in the classname. If your test intentionally includes this character as part of its name, remove it. This character is reserved for encoding file paths in the classname attribute." + ); + // If there is an error, continue trying to process other files. + continue; + } + + const maybeSpecFilePath: Result = findSpecFilePathFromRootSuite(reportJson); + + if ('error' in maybeSpecFilePath) { + logError(maybeSpecFilePath.error); + // If there is an error, continue trying to process other files. + continue; + } + + const reportString: string = await transformedReport({ + reportJson, + specFilePath: maybeSpecFilePath.result, + reportName: flags.reportName, + rootDirectory: flags.rootDirectory, + }); + + // If the writeInPlace flag was passed, overwrite the original file, otherwise log the output to stdout + if (flags.writeInPlace) { + log.info(`Wrote transformed junit report to ${path}`); + await fs.writeFile(path, reportString); + } else { + log.write(reportString); + } + } + log.success('task complete'); +} + +/** + * The args passed to our command. These are a subset of RunContext. By using a subset, we make mocking easier. + */ +export interface CommandArgs { + flags: { [index: string]: string | boolean | string[] | undefined }; + /** just pick the parts of `log` that we use. This makes mocking much easier. */ + log: Pick; +}