phenomnomnominal / betterer

betterer makes it easier to make incremental improvements to your codebase
MIT License
576 stars 38 forks source link

Betterer CI fails #1124

Open MFredX opened 1 year ago

MFredX commented 1 year ago

Describe the bug npm run betterer ci failing in my Azure Pipelines CI

To Reproduce When I ran npm run betterer ci on my pipeline step, the following error was observed.

   \ | /     _         _   _
 '-.ooo.-'  | |__  ___| |_| |_ ___ _ __ ___ _ __
---ooooo--- | '_ \/ _ \ __| __/ _ \ '__/ _ \ '__|
 .-'ooo'-.  | |_)|  __/ |_| ||  __/ | |  __/ |
   / | \    |_.__/\___|\__|\__\___|_|  \___|_|

πŸŽ‰ Betterer: 1 test done!
βœ… strictNullChecks compilation: "strictNullChecks compilation" stayed the same.
 (2,674 issues) 😐

1 test got checked. πŸ€”
1 test stayed the same. 😐

Unexpected changes detected in these tests while running in CI mode:

 "strictNullChecks compilation"

You should make sure the results file is up-to-date before committing! You might
 want to run `betterer precommit` in a commit hook. πŸ’

However, when I run npm run betterer ci in my local directory, the command is executed successfully. When I push to remote, I push the most up-to-date results file.

It appears there are a few more users who faced the same issue as per; https://github.com/phenomnomnominal/betterer/issues/983

Expected behavior I expect the Betterer CI command to work on my Azure CI pipeline the way it behaves in my local

Any advice on how I can debug this on the CI?

Versions (please complete the following information):

hya15 commented 1 year ago

I hope this is helpful: I diffed the baseline / result structures under runSummaries in suiteSummary and found several types of issues on CircleCI. My dependencies are pinned to specific versions in package.json. CI runs npm ci as part of its pipeline. While debugging this issue, I ran npm ci before generating a new .betterer.results file.

A temporary fix may be to patch your local installation to not fail on changes if the corresponding runSummary's isSame property is true, until solutions addressing root cause(es) are found.

1. Differing file paths for various error messages

baseline:

"Could not find a declaration file for module 'urijs'. 'local_project/node_modules/urijs/src/URI.js' implicitly has an 'any' type.\n    Try `npm i --save-dev @types/urijs` if it exists or add a new declaration (.d.ts) file containing `declare module 'urijs';`",

results:

"Could not find a declaration file for module 'urijs'. '/home/circleci/project/node_modules/urijs/src/URI.js' implicitly has an 'any' type.\n    Try `npm i --save-dev @types/urijs` if it exists or add a new declaration (.d.ts) file containing `declare module 'urijs';`",

baseline:

"Type 'undefined' is not assignable to type 'RenderResult<typeof import(\"local_project/node_modules/@testing-library/dom/types/queries\")>'.",

results:

"Type 'undefined' is not assignable to type 'RenderResult<typeof import(\"/home/circleci/project/node_modules/@testing-library/dom/types/queries\")>'.",

2. Type definitions reordered:

This was the bulk of the differences between baseline and result.

baseline:

"Argument of type 'HTMLElement | null' is not assignable to parameter of type 'Window | Document | Node | Element'.\n    Type 'null' is not assignable to type 'Window | Document | Node | Element'.",

result:

"Argument of type 'HTMLElement | null' is not assignable to parameter of type 'Document | Node | Element | Window'.\n    Type 'null' is not assignable to type 'Document | Node | Element | Window'.",

TS's truncated messages show different fields:

baseline:

"Type '{ id: \"empty\"; }' is missing the following properties from type 'IDiscountDisplay': discountCode, type, status, discountAmount, and 3 more.",

result:

"Type '{ id: \"empty\"; }' is missing the following properties from type 'IDiscountDisplay': discountCode, type, status, discountMax, and 3 more.",
MFredX commented 1 year ago

Thanks for that lead, upon further examination at my results files I see that there some differences in my local results file and the results file on my CI.

This pattern below was noticed in several places across the results file.

local results file

[77, 8, 3, "Type \'import(\\"product/ui/src/app/enums/my-enum.enum\\").MyEnum\' is not assignable to type \'never\'.", "193424690"],

results file on my CI

  [77, 8, 3, "Type \'import(\\"s/ui/src/app/enums/my-enum.enum\\").MyEnum\' is not assignable to type \'never\'.", "193424690"],

I can confirm that the differences in the file are caused by the names of the CI specific build folders being appended to the errors. The s directory was where are application was present in the CI pipeline, this is Azure specific.

MFredX commented 1 year ago

Our team did some investigation to find out on what causes the different file paths for certain TypeScript errors. It turns out that certain TypeScript errors have full path names referred in the error messages. This causes differences between the Betterer results file locally and on the CI.

This issue is an open issue in TypeScript as of today; https://github.com/microsoft/TypeScript/issues/41398

10/05/2024 EDIT - We fixed our failures by actually fixing the Betterer errors that caused the differences in both files. Not the neatest way forward but it did the trick for us.

phenomnomnominal commented 1 year ago

Yeah, I have some code which is meant to strip out most of these, but it might not be smart enough! Hopefully can be done without adding to the API, but might require a custom transform hook before writing the results file

finalight commented 1 year ago

i am still facing the same issue

 "@betterer/betterer": "^5.4.0",
    "@betterer/cli": "^5.4.0",
    "@betterer/eslint": "^5.4.0",
    "@betterer/typescript": "^5.4.0",
work933k commented 7 months ago

We are also having this issue in our CI-pipeline. We are using the nx-betterer plugin and reverted back to v1.1.5 from v1.2.1. I just noticed that a new version v2.0.2 has become available. Maybe that resolves the issue for our codebase.