Skip to content

fix: output report generation#355

Merged
ricardogobbosouza merged 2 commits intowebpack:masterfrom
grantcox:fix-outputreport-formatter
May 22, 2024
Merged

fix: output report generation#355
ricardogobbosouza merged 2 commits intowebpack:masterfrom
grantcox:fix-outputreport-formatter

Conversation

@grantcox
Copy link
Copy Markdown
Contributor

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

The changes in #300 broke the outputReport functionality, specifically:

-      const content = outputReport.formatter
-        ? loadFormatter(stylelint, outputReport.formatter)(results)
-        : formatter(results);
+      const content = outputReport.formatter;
+      loadFormatter(stylelint, outputReport.formatter)(results, returnValue);
+      formatter(results, returnValue);

means that the content that is written to the output file, is always just the .toString() of the outputReport.formatter itself. For example:

outputReport: {
  filePath: "stylelint-report.json"
  formatter: "json",
}

> cat stylelint-report.json
json

I have not added additional tests for this change, as I am unable to run the existing test suite:

node --version
v18.16.1
npm install
npm WARN deprecated babel-eslint@10.1.0: babel-eslint is now @babel/eslint-parser. This package will no longer receive updates.

> stylelint-webpack-plugin@5.0.0 prepare
> husky && npm run build


> stylelint-webpack-plugin@5.0.0 prebuild
> npm run clean


> stylelint-webpack-plugin@5.0.0 clean
> del-cli dist types


> stylelint-webpack-plugin@5.0.0 build
> npm-run-all -p "build:**"


> stylelint-webpack-plugin@5.0.0 build:types
> tsc --declaration --emitDeclarationOnly --outDir types && prettier "types/**/*.ts" --write


> stylelint-webpack-plugin@5.0.0 build:code
> cross-env NODE_ENV=production babel src -d dist --copy-files

Successfully compiled 7 files with Babel (173ms).
types/getStylelint.d.ts 106ms
types/index.d.ts 6ms
types/linter.d.ts 7ms
types/options.d.ts 5ms
types/StylelintError.d.ts 1ms
types/utils.d.ts 4ms
types/worker.d.ts 1ms (unchanged)

added 1282 packages, and audited 1283 packages in 6s

218 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities
npm run test:only

...
 FAIL  test/emit-error.test.js
  ● Test suite failed to run

    A jest worker process (pid=88843) was terminated by another process: signal=SIGSEGV, exitCode=null. Operating system logs may contain more information on why this occurred.

      at ChildProcessWorker._onExit (node_modules/jest-worker/build/workers/ChildProcessWorker.js:370:23)

 FAIL  test/stylelint-ignore.test.js
  ● Test suite failed to run

    A jest worker process (pid=88842) was terminated by another process: signal=SIGSEGV, exitCode=null. Operating system logs may contain more information on why this occurred.

      at ChildProcessWorker._onExit (node_modules/jest-worker/build/workers/ChildProcessWorker.js:370:23)

 FAIL  test/output-report.test.js
  ● Test suite failed to run

    A jest worker process (pid=88844) was terminated by another process: signal=SIGSEGV, exitCode=null. Operating system logs may contain more information on why this occurred.

      at ChildProcessWorker._onExit (node_modules/jest-worker/build/workers/ChildProcessWorker.js:370:23)


Test Suites: 15 failed, 9 passed, 24 total
Tests:       19 passed, 19 total
Snapshots:   6 passed, 6 total
Time:        3.327 s
Ran all test suites.

Please note that the existing output-report.test.js test cases do not assert the content of the output file at all, just that it was created. This will be why the tests did not catch this regression when it occurred.

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented May 17, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: grantcox / name: Grant Cox (c3dba49)
  • ✅ login: ricardogobbosouza / name: Ricardo Gobbo de Souza (a826650)

@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (48ddc4e) to head (a826650).
Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #355   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          246       247    +1     
  Branches        50        54    +4     
=========================================
+ Hits           246       247    +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ricardogobbosouza ricardogobbosouza changed the title Fix outputReport generation fix: outputReport generation May 21, 2024
@ricardogobbosouza ricardogobbosouza changed the title fix: outputReport generation fix: output report generation May 21, 2024
@ricardogobbosouza ricardogobbosouza force-pushed the fix-outputreport-formatter branch from 610aea5 to c3dba49 Compare May 21, 2024 14:17
@ricardogobbosouza
Copy link
Copy Markdown
Collaborator

I added a test to cover the content of the output report

@ricardogobbosouza ricardogobbosouza merged commit d5d4ac1 into webpack:master May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants