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

[INFRA] Add SonarCloud analysis on CI build #906

Merged
merged 29 commits into from
Nov 30, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
a57bce3
Add configuration to run a SonarCloud analysis on master branch and p…
csouchet Nov 25, 2020
b50b176
Add the configuration file in the root directory of the project: sona…
csouchet Nov 25, 2020
78ccba8
- Generate eslint report in json format
csouchet Nov 25, 2020
263ac74
Generate Jest test execution report & configure Sonar
csouchet Nov 25, 2020
410ea9f
Execute Sonar analysis after all steps of build and only on ubuntu
csouchet Nov 25, 2020
2be0664
Fix coverage generation
csouchet Nov 25, 2020
62fadcf
Configure Sonar to get the coverage reports
csouchet Nov 25, 2020
05d44ca
add coverage badge in README
csouchet Nov 25, 2020
074bdf8
Run coverage only for ubuntu
csouchet Nov 25, 2020
7d79cb8
Add a global property for coverage
csouchet Nov 25, 2020
47e9dc0
Remove Eslint configuration for Sonar, because the job failed, if the…
csouchet Nov 26, 2020
09731fc
Use the 1.4 version of the Sonar Github Action
csouchet Nov 26, 2020
bafe09b
Remove the coverage thresholds
csouchet Nov 26, 2020
e12e44d
add 'src/model' to ignored path for the coverage
csouchet Nov 26, 2020
eb3e37f
remove unnecessary type of pull request on github workflow
csouchet Nov 26, 2020
246bb0e
Remove source config for Sonar
csouchet Nov 26, 2020
201f771
Change path of source & test for Sonar config
csouchet Nov 26, 2020
e78c05c
Fix source encoding for Sonar
csouchet Nov 26, 2020
6fd6757
exclude jest config files for the Sonar analysis
csouchet Nov 26, 2020
f1b2e1c
Eclude model from Sonar analysis
csouchet Nov 26, 2020
7574e12
Disable shallow clone for Sonar
csouchet Nov 26, 2020
168a12b
Disable tsconfig
csouchet Nov 26, 2020
1ad461d
Disable src for Sonar config
csouchet Nov 26, 2020
9b4e874
Enable src for Sonar config & change exclusions
csouchet Nov 26, 2020
747b625
Config works in local
csouchet Nov 26, 2020
d3cac9d
Exclude files from src/model, src/demo and src/static
csouchet Nov 27, 2020
0880bdc
Apply suggestions from code review
csouchet Nov 27, 2020
ec18173
Fix ignore test paths on test:e2e:coverage
csouchet Nov 27, 2020
ff38e70
Merge branch 'master' into 823-Add_code_coverage_public_reports
csouchet Nov 30, 2020
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
24 changes: 20 additions & 4 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,19 @@ jobs:
matrix:
# syntax inspired from https://github.community/t5/GitHub-Actions/Using-a-matrix-defined-input-for-a-custom-action/m-p/32032/highlight/true#M988
os:
- { name: ubuntu-20.04 }
- { name: ubuntu-20.04, coverage: '-- --coverage' }
- { name: macos-10.15 }
- { name: windows-2019 }
steps:
- uses: actions/checkout@v2
- name: Checkout with shallow clone
uses: actions/checkout@v2
if: ${{ !contains(matrix.os.coverage, 'coverage') }}
- name: Checkout without shallow clone
uses: actions/checkout@v2
if: ${{ contains(matrix.os.coverage, 'coverage') }}
with:
# Disabling shallow clone is recommended for improving relevancy of SonarCloud reporting
fetch-depth: 0
- name: Setup node
uses: actions/setup-node@v1
with:
Expand All @@ -54,7 +62,7 @@ jobs:
run: npm run build
- name: Test Application
id: 'test_unit'
run: npm run test:unit
run: npm run test:unit ${{ matrix.os.coverage }}
Copy link
Member

@tbouffard tbouffard Nov 26, 2020

Choose a reason for hiding this comment

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

good way to configure the step according to the os! I am not this is the best name, for now, it is ok (I don't have any best proposal 😸)

- name: Upload unit test results
if: ${{ failure() && steps.test_unit.outcome == 'failure' }}
uses: actions/upload-artifact@v2
Expand All @@ -63,7 +71,7 @@ jobs:
path: build/test-report/unit
- name: Test Application End to End
id: 'test_e2e'
run: npm run test:e2e
run: npm run test:e2e ${{ matrix.os.coverage }}
- name: Upload e2e test results
if: ${{ failure() && steps.test_e2e.outcome == 'failure' }}
uses: actions/upload-artifact@v2
Expand All @@ -73,3 +81,11 @@ jobs:
# Ensure we don't break scripts
- name: Build utils
run: npm run build-utils

# No need to run the analysis from all environments
- name: SonarCloud Scan
if: ${{ success() && contains(matrix.os.coverage, 'coverage') }}
uses: SonarSource/[email protected]
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} # Needed to get PR information, if any
SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }}
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
<a href="https://github.com/process-analytics/bpmn-visualization-js/actions">
<img alt="Build" src="https://github.com/process-analytics/bpmn-visualization-js/workflows/Build/badge.svg">
</a>
<a href="https://sonarcloud.io/dashboard?id=process-analytics_bpmn-visualization-js">
<img alt="Coverage" src="https://sonarcloud.io/api/project_badges/measure?project=process-analytics_bpmn-visualization-js&metric=coverage">
Copy link
Member

Choose a reason for hiding this comment

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

👍 for the badge

image

</a>
<a href="https://gitpod.io/#https://github.com/process-analytics/bpmn-visualization-js" target="_blank">
<img alt="Gitpod" src="https://img.shields.io/badge/Gitpod-ready--to--code-chartreuse?logo=gitpod">
</a>
Expand Down
18 changes: 18 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@
"lint-check": "tsc --noEmit && eslint \"*/**/*.{js,ts,tsx}\" NOTICE --max-warnings 0",
"test": "run-s test:unit test:e2e",
"test:unit": "jest --runInBand --config=./test/unit/jest.config.js",
"test:unit:coverage": "jest --runInBand --config=./test/unit/jest.config.js --coverage",
"test:unit:watch": "jest --runInBand --config=./test/unit/jest.config.js --coverage --watchAll",
"test:unit:coverage": "npm run test:unit -- --coverage",
Copy link
Member

Choose a reason for hiding this comment

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

❓ did you try run-s instead of npm (which probably fork a new process). This applies to all scripts that have been chandeg to update another script.

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't think to do that ^^

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it's adapted to what we want to do.
Here we just want to run the same configuration as for the unit test, but with one more paramter (for the coverage).

I tried with run-s. It doesn't work. After the documentation reading of run-s, it's more to run mutliple scripts (it's not what we do).

Copy link
Member

Choose a reason for hiding this comment

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

So let's keep it

"test:unit:watch": "npm run test:unit:coverage -- --watchAll",
Copy link
Member

Choose a reason for hiding this comment

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

⚠️ behaviour change
I don't use this command (nor know why we created it) so I won't be impacted.
If @aibcmars is ok with this change, let's do it (check the doc if there is any mention to it)

Copy link
Member

Choose a reason for hiding this comment

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

Actually, the coverage was already run as part of the 'watch', so there is no behaviour change.

"test:e2e": "cross-env DEBUG=test JEST_IMAGE_SNAPSHOT_TRACK_OBSOLETE=1 JEST_PUPPETEER_CONFIG=./test/e2e/jest-puppeteer.config.js jest --runInBand --detectOpenHandles --testPathIgnorePatterns ./test/e2e/performance --config=./test/e2e/jest.config.js",
"test:perf": "cd ./test/e2e && jest --runInBand --detectOpenHandles --testNamePattern=performance",
"test:e2e:coverage": "cross-env JEST_IMAGE_SNAPSHOT_TRACK_OBSOLETE=1 jest --runInBand --detectOpenHandles --config=./test/e2e/jest.config.js --coverage"
"test:e2e:coverage": "npm run test:e2e -- --coverage"
},
"dependencies": {
"entities": "^2.1.0",
Expand Down Expand Up @@ -103,6 +103,7 @@
"jest-html-reporter": "^3.3.0",
"jest-image-snapshot": "^4.2.0",
"jest-puppeteer": "^4.4.0",
"jest-sonar": "^0.2.11",
Copy link
Member

Choose a reason for hiding this comment

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

❓ do you know if this has perf impact on our local dev i.e if you have seen processing time increase when running tests?
I will do some checks on my side

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't check yet.

Copy link
Member Author

@csouchet csouchet Nov 27, 2020

Choose a reason for hiding this comment

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

Ubuntu

Before jest-sonar integration

  • Unit test: 22s
  • E2e test: 2m 22s

After jest-sonar integration

  • Unit test: 26s
  • E2e test: 2m 26s

MacOS

Before jest-sonar integration

  • Unit test: 28s
  • E2e test: 3m 32s

After jest-sonar integration

  • Unit test: 25s
  • E2e test: 3m 22s

Windows

Before jest-sonar integration

  • Unit test: 31s
  • E2e test: 3m 32s

After jest-sonar integration

  • Unit test: 38s
  • E2e test: 3m 59s

Copy link
Member

Choose a reason for hiding this comment

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

Ok, thanks for the figures
It looks ok

"lint-staged": "^10.5.2",
"minimist": "^1.2.5",
"mxgraph-type-definitions": "^1.0.4",
Expand Down
29 changes: 29 additions & 0 deletions sonar-project.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Organization and project keys are displayed in the right sidebar of the project homepage
sonar.projectKey=process-analytics_bpmn-visualization-js
sonar.organization=process-analytics

# This is the name and version displayed in the SonarCloud UI.
sonar.projectName=bpmn-visualization
sonar.projectVersion=0.7.0

# Path is relative to the sonar-project.properties file. Replace "\" by "/" on Windows.
sonar.sources=src
sonar.exclusions=src/model/**/*,src/demo/**/*,src/static/**/*
#sonar.inclusions=
# Encoding of the source code. Default is default system encoding
sonar.sourceEncoding=UTF-8

# Path to tests
sonar.tests=test
sonar.test.exclusions=**/jest.config.js,**/*.png
#sonar.test.inclusions=

sonar.javascript.lcov.reportPaths=build/test-report/unit/lcov.info,build/test-report/e2e/lcov.info
sonar.testExecutionReportPaths=build/test-report/unit/sonar-report.xml,build/test-report/e2e/sonar-report.xml

# The job failed before the sonar analysis, if the lint check failed. So no need to configure eslint for Sonar
#sonar.eslint.reportPaths=build/eslint-reporter.json
sonar.typescript.tsconfigPath=tsconfig.json

# Exclusions for copy-paste detection
#sonar.cpd.exclusions=
19 changes: 15 additions & 4 deletions test/e2e/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,25 @@
* limitations under the License.
*/
module.exports = {
rootDir: '../..',
roots: ['./test/e2e', './src'],
preset: 'jest-puppeteer',
roots: ['./'],
testMatch: ['**/?(*.)+(spec|test).[t]s'],
testPathIgnorePatterns: ['/node_modules/', 'dist'],
testPathIgnorePatterns: ['/node_modules/', 'dist', 'src'],
testTimeout: 200000,
transform: {
'^.+\\.ts?$': 'ts-jest',
},
testEnvironment: 'jest-environment-puppeteer-jsdom',
globalSetup: 'jest-environment-puppeteer-jsdom/setup',
globalTeardown: 'jest-environment-puppeteer-jsdom/teardown',
setupFiles: ['./config/jest.globals.ts'],
collectCoverageFrom: ['**/*.{ts,js}'],
coveragePathIgnorePatterns: ['/node_modules/', 'dist', 'test', 'src/demo', 'src/static', 'src/model'],
coverageReporters: ['lcovonly', 'text', 'text-summary'],
coverageDirectory: 'build/test-report/e2e',
setupFiles: ['./test/e2e/config/jest.globals.ts'],
// jest-image-snapshot configuration doesn't work with setupFiles, fix with setupFilesAfterEnv: see https://github.com/testing-library/jest-dom/issues/122#issuecomment-650520461
setupFilesAfterEnv: ['./config/jest.image.ts'],
setupFilesAfterEnv: ['./test/e2e/config/jest.image.ts'],
reporters: [
'default',
[
Expand All @@ -39,5 +44,11 @@ module.exports = {
includeSuiteFailure: true,
},
],
[
'jest-sonar',
{
outputDirectory: 'build/test-report/e2e',
},
],
],
};
26 changes: 13 additions & 13 deletions test/unit/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,23 @@
* limitations under the License.
*/
module.exports = {
roots: ['./'],
rootDir: '../..',
roots: ['./test/unit', './src'],
moduleNameMapper: {
// mock files that jest doesn't support like CSS and SVG files
'\\.css$': './../module-mock.js',
'\\.svg$': './../module-mock.js',
},
testMatch: ['**/?(*.)+(spec|test).[t]s'],
testPathIgnorePatterns: ['/node_modules/', 'dist'],
testPathIgnorePatterns: ['/node_modules/', 'dist', 'src'],
transform: {
'^.+\\.ts?$': 'ts-jest',
},
collectCoverageFrom: ['**/*.{ts,js}'],
coveragePathIgnorePatterns: ['/node_modules/', 'dist', 'test'],
coverageThreshold: {
global: {
branches: 80,
functions: 80,
lines: 80,
statements: 80,
},
},
Comment on lines -30 to -37
Copy link
Member

Choose a reason for hiding this comment

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

❓ I guess this disable the 'fail build' detection when we are under the threshold?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes ^^

coverageReporters: ['json', 'json-summary', 'lcov', 'text', 'text-summary', 'clover'],
setupFiles: ['./jest.globals.ts'],
coveragePathIgnorePatterns: ['/node_modules/', 'dist', 'test', 'src/demo', 'src/static', 'src/model'],
coverageReporters: ['lcovonly', 'text', 'text-summary'],
coverageDirectory: 'build/test-report/unit',
setupFiles: ['./test/unit/jest.globals.ts'],
reporters: [
'default',
[
Expand All @@ -48,5 +42,11 @@ module.exports = {
includeSuiteFailure: true,
},
],
[
'jest-sonar',
{
outputDirectory: 'build/test-report/unit',
},
],
],
};