Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tools: Product-specific visual comparison #19990

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ test/cypress
test/test-*.*
test/*.ts
test/typescript-karma
tools/affected-product
tools/eslint-plugin-highcharts/node_modules/**
tools/gulptasks/dashboards/api-docs/**/*.js
tools/jsdoc/storage/**/*.js
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,6 @@ jobs:
- name: Build Highcharts
run: npx gulp scripts

- name: Build Dashboards
run: npx gulp dashboards/scripts

- name: Write file sizes at master
run: npx gulp write-file-sizes --filename master.json

Expand Down Expand Up @@ -85,6 +82,7 @@ jobs:
- uses: actions/checkout@v4
with:
ref: master
fetch-depth: 2

- name: Use Node.js ${{ matrix.node-version }}
uses: actions/setup-node@v3
Expand All @@ -98,10 +96,10 @@ jobs:
- name: Build Highcharts
run: npx gulp scripts

- run: npx karma start test/karma-conf.js --tests highcharts/*/* --reference --browsercount 2 --no-fail-on-empty-test-suite
- run: npx karma start test/karma-conf.js --tests maps/*/* --reference --browsercount 2 --no-fail-on-empty-test-suite
- run: npx karma start test/karma-conf.js --tests stock/*/* --reference --browsercount 2 --no-fail-on-empty-test-suite
- run: npx karma start test/karma-conf.js --tests gantt/*/* --reference --browsercount 2 --no-fail-on-empty-test-suite
- run: npx karma start test/karma-conf.js --iff Core,Data,Series,Shared --tests highcharts/*/* --reference --browsercount 2 --no-fail-on-empty-test-suite
- run: npx karma start test/karma-conf.js --iff Stock,Data,Series,Shared --tests stock/*/* --reference --browsercount 2 --no-fail-on-empty-test-suite
- run: npx karma start test/karma-conf.js --iff Maps,Data,Series,Shared --tests maps/*/* --reference --browsercount 2 --no-fail-on-empty-test-suite
- run: npx karma start test/karma-conf.js --iff Gantt,Data,Series,Shared --tests gantt/*/* --reference --browsercount 2 --no-fail-on-empty-test-suite

- name: Checkout current branch
uses: actions/checkout@v4
Expand All @@ -121,10 +119,10 @@ jobs:
- name: Build Highcharts
run: npx gulp scripts

- run: npx karma start test/karma-conf.js --tests highcharts/*/* --single-run --browsercount 2 --visualcompare || true
- run: npx karma start test/karma-conf.js --tests stock/*/* --single-run --browsercount 2 --visualcompare || true
- run: npx karma start test/karma-conf.js --tests maps/*/* --single-run --browsercount 2 --visualcompare || true
- run: npx karma start test/karma-conf.js --tests gantt/*/* --single-run --browsercount 2 --visualcompare || true
- run: npx karma start test/karma-conf.js --iff Core,Data,Series,Shared --tests highcharts/*/* --single-run --browsercount 2 --visualcompare || true
- run: npx karma start test/karma-conf.js --iff Stock,Data,Series,Shared --tests stock/*/* --single-run --browsercount 2 --visualcompare || true
- run: npx karma start test/karma-conf.js --iff Maps,Data,Series,Shared --tests maps/*/* --single-run --browsercount 2 --visualcompare || true
- run: npx karma start test/karma-conf.js --iff Gantt,Data,Series,Shared --tests gantt/*/* --single-run --browsercount 2 --visualcompare || true

- name: Save test results where both an reference.svg and candidate.svg exists
run: |
Expand All @@ -146,7 +144,6 @@ jobs:
AWS_SECRET_ACCESS_KEY: ${{secrets.VISUAL_TESTS_AWS_SECRET_ACCESS_KEY}}
AWS_REGION: ${{secrets.VISUAL_TESTS_AWS_REGION}}


- name: Store artifacts
uses: actions/upload-artifact@v3
if: ${{ always() }}
Expand Down
6 changes: 4 additions & 2 deletions cypress.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ for (const product of getProducts()) {
}

const productsPattern = (
products.length ?
products.length > 1 ?
`{${Array.from(products).join(',')}}` :
'**'
products.length ?
products[0] :
'**'
);

export default defineConfig({
Expand Down
18 changes: 18 additions & 0 deletions test/karma-conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const yaml = require('js-yaml');
const path = require('path');
const os = require('os');
const { getLatestCommitShaSync } = require('../tools/gulptasks/lib/git');
const { getProducts } = require('../tools/gulptasks/lib/test');

const VISUAL_TEST_REPORT_PATH = 'test/visual-test-results.json';
const version = require('../package.json').version;
Expand Down Expand Up @@ -169,6 +170,23 @@ module.exports = function (config) {
const argv = require('yargs').argv;
const Babel = require("@babel/core");

if (argv.iff) {
const folders = argv.iff.split(',');
const products = getProducts(false, true);

let affected = false;

for (const folder of folders) {
if (products.includes(folder)) {
affected = true;
}
}

if (!affected) {
return process.exit();
}
}

if (argv.ts) {
const ChildProcess = require('child_process');
// Compile test tools and samples
Expand Down
59 changes: 22 additions & 37 deletions tools/gulptasks/lib/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,79 +17,65 @@
/**
* Returns list of products affected by modified files staged for commit.
*
* @param {Boolean} logPaths
* @param {boolean} logPaths
* Logging for testing and debugging.
*
* @return {Array<string>}
* Array of detected products.
*/
function getProducts(logPaths) {
const ChildProcess = require('child_process');

const paths = ChildProcess
.execSync('git diff --cached --name-only --diff-filter=ACM')
.toString()
.split('\n')
.filter(match => !!match),
const ChildProcess = require('node:child_process');

const paths = Array.from(new Set([
...ChildProcess
.execSync('git diff --cached --name-only --diff-filter=ACM')
.toString()
.split('\n')
.filter(match => !!match),
...ChildProcess
.execSync('git diff refs/heads/master HEAD --name-only --diff-filter=ACM')
Copy link
Member

@goransle goransle Nov 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still see some git weirdness here.
image

Locally this worked for me on a single-branch checkout clone:

git remote set-branches origin 'master'
git fetch -v --depth=1
git diff HEAD origin/master

.toString()
.split('\n')
.filter(match => !!match)
])),
products = [
'Core',
'Dashboards',
'Gantt',
'Maps',
'Stock'
],
affectedProducts = [];
affectedProducts = new Set();

// Logging for testing and debugging
if (logPaths) {
const log = require('./log');
log.message('paths: ', paths);
}

function mark(product) {
if (affectedProducts.indexOf(product) !== -1) {
affectedProducts.push(product);
}
}

paths.forEach(path => {
// Any path part check
products.forEach(productName => {
const productNameRegex = new RegExp(productName, 'iu');
if (productNameRegex.test(path)) {
mark(productName);
affectedProducts.add(productName);
}
});

// By directory detection
const pathParts = path.split('/');

if (pathParts.length > 2 && pathParts[0] === 'ts') {
if (['Shared', 'Data'].indexOf(pathParts[1]) !== -1) {
mark('Core');
mark('Dashboards');
if (['Accessibility', 'Series', 'Shared', 'Data'].indexOf(pathParts[1]) !== -1) {
affectedProducts.add('Core');
affectedProducts.add('Dashboards');
} else if (pathParts[1] === 'DataGrid') {
mark('Dashboards');
affectedProducts.add('Dashboards');
}
}
});

return affectedProducts;
}

/**
* Fails if the product is not affected in the files staged for commit.
*
* @throws Will throw an error if the product is not affected.
*
* @param {string} product
* The product name that should be checked.
*/
function checkProduct(product) {
const products = getProducts();
if (products.indexOf(product) === -1) {
throw new Error(`${product} is not affected`);
}
return Array.from(affectedProducts);
}

/**
Expand Down Expand Up @@ -200,7 +186,6 @@ function saveRun({
* */

module.exports = {
checkProduct,
getProducts,
shouldRun,
saveRun
Expand Down
Loading