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

linting types disabled (too slow) #5788

Closed
turadg opened this issue Jul 20, 2022 · 3 comments · Fixed by #8058
Closed

linting types disabled (too slow) #5788

turadg opened this issue Jul 20, 2022 · 3 comments · Fixed by #8058
Labels
bug Something isn't working tooling repo-wide infrastructure

Comments

@turadg
Copy link
Member

turadg commented Jul 20, 2022

What is the Problem Being Solved?

The lint test sometimes fails even though there are no errors. The output shows no lint errors and running lint locally passes, but the exit code is 1.

✖ 54 problems (0 errors, 54 warnings)
lerna success - @agoric/ui-components
lerna success - @agoric/vat-data
lerna success - @agoric/wallet-connection
lerna success - @agoric/web-components
lerna success - @agoric/xsnap
lerna success - @agoric/wallet-backend
lerna success - @agoric/wallet-ui
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Error: Process completed with exit code 1.

Some instances:
https://github.com/Agoric/agoric-sdk/runs/7430249641?check_suite_focus=true

They're all in lint-rest which has grown from 9m when typed linting was enabled to 30min in a recent run. @agoric/wallet-backend alone took 20min. I did some investigation but after a while gave up as it's not a priority. It was only producing warnings and we've resolved many of them. The rest will probably take a ticket to resolve and in that one we can enable the flag locally to get all the warnings.

So for now it's disabled. 4a84690

Description of the Design

No diagnosis yet.

Security Considerations

Test Plan

@turadg turadg added bug Something isn't working tooling repo-wide infrastructure labels Jul 20, 2022
@turadg
Copy link
Member Author

turadg commented Jul 20, 2022

I think it's timing out. lint-primary is 9 min and lint-rest used to be but now it's up to 30m:
https://github.com/Agoric/agoric-sdk/runs/7430971504?check_suite_focus=true

I suspect it's due to some new pathology in wallet-backend:

lerna info run Ran npm script 'lint' in '@agoric/wallet-backend' in 1198.2s:
> @agoric/wallet-backend@0.12.1 lint
> run-s --continue-on-error lint:*
> @agoric/wallet-backend@0.12.1 lint:types
> tsc --maxNodeModuleJsDepth 3 -p jsconfig.json
> @agoric/wallet-backend@0.12.1 lint:eslint
> eslint .

That takes 30s on my M1 (with AGORIC_ESLINT_TYPES=true).

@turadg
Copy link
Member Author

turadg commented Jul 20, 2022

Disabling for now since we have reports on the type warnings and none of them are errors yet. 4a84690

job AGORIC_ESLINT_TYPES disabled
lint-primary 9 min 3 min
lint-rest 30 min 4 min

I'm updating the title of the ticket. When we triage tooling work we can decide whether to solve the perf problem and reenable.

@turadg turadg changed the title CI lint sometimes fails erroneously linting types disabled (too slow) Jul 20, 2022
@turadg
Copy link
Member Author

turadg commented Jan 16, 2023

We could make it faster by doing this slow types-based linting only on the files that have changed since master. And it could be a non-required check so that PRs could still merge.

Doing so will require #4645

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tooling repo-wide infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants