fix(biome.js) :: fix remaining lint issues - #1366
Conversation
|
More validations including screenshots. LMK if there's any comments/questions/concerns and I will address them :) $ npm test
> sqlpage@1.0.0 test
> biome check .
Checked 36 files in 23ms. No fixes applied.$ cargo build
Compiling sqlpage v0.45.0 (/home/reap/Desktop/SQLPage)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 18.82s$ cargo test
Compiling sqlpage v0.45.0 (/home/reap/Desktop/SQLPage)
Finished `test` profile [unoptimized + debuginfo] target(s) in 15.15s
Running unittests src/lib.rs (target/debug/deps/sqlpage-9c0b80f3c9b03581)
running 175 tests
[...]
test result: ok. 175 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.47s
Running unittests src/main.rs (target/debug/deps/sqlpage-6d93c28a6118a58e)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s
Running tests/mod.rs (target/debug/deps/mod-6dd7f46546fbaa06)
running 72 tests
[...]
test result: ok. 72 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 3.29s
Doc-tests sqlpage
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s$ cd examples/official-site
$ SQLPAGE_PORT=8099 ../../target/debug/sqlpage 
|
|
I did also run integration tests. All but one of them passed on my dev machine. The one that fails seems to be a cert issue unrelated to my changes. I will defer to the automated CI runs to confirm. @lovasoa would you be able to start the workflow for me? $ npx playwright test
Running 22 tests using 8 workers
[chromium] › official-site.spec.ts:21:5 › chart
Skipping host requirements validation logic because `PLAYWRIGHT_SKIP_VALIDATE_HOST_REQUIREMENTS` env variable is set.
[chromium] › official-site.spec.ts:233:5 › form example
Skipping host requirements validation logic because `PLAYWRIGHT_SKIP_VALIDATE_HOST_REQUIREMENTS` env variable is set.
[chromium] › official-site.spec.ts:264:5 › Authentication example
Skipping host requirements validation logic because `PLAYWRIGHT_SKIP_VALIDATE_HOST_REQUIREMENTS` env variable is set.
[chromium] › official-site.spec.ts:44:5 › toast notifications initialize, stack, dismiss, and render safely
Skipping host requirements validation logic because `PLAYWRIGHT_SKIP_VALIDATE_HOST_REQUIREMENTS` env variable is set.
[chromium] › official-site.spec.ts:38:5 › map
Skipping host requirements validation logic because `PLAYWRIGHT_SKIP_VALIDATE_HOST_REQUIREMENTS` env variable is set.
[chromium] › official-site.spec.ts:27:5 › chart supports hiding legend
Skipping host requirements validation logic because `PLAYWRIGHT_SKIP_VALIDATE_HOST_REQUIREMENTS` env variable is set.
[chromium] › official-site.spec.ts:5:5 › Open documentation
Skipping host requirements validation logic because `PLAYWRIGHT_SKIP_VALIDATE_HOST_REQUIREMENTS` env variable is set.
[chromium] › official-site.spec.ts:247:5 › File upload
Skipping host requirements validation logic because `PLAYWRIGHT_SKIP_VALIDATE_HOST_REQUIREMENTS` env variable is set.
1) [chromium] › official-site.spec.ts:356:5 › no console errors on card page ─────────────────────
Error: expect(received).toHaveLength(expected)
Expected length: 0
Received length: 2
Received array: ["Failed to load resource: net::ERR_CERT_AUTHORITY_INVALID", "Failed to load resource: net::ERR_CERT_AUTHORITY_INVALID"]
339 | await page.waitForLoadState();
340 |
> 341 | expect(errors).toHaveLength(0);
| ^
342 | }
343 |
344 | test("no console errors on table page", async ({ page }) => {
at checkNoConsoleErrors (/home/reap/Desktop/SQLPage/tests/end-to-end/official-site.spec.ts:341:18)
at /home/reap/Desktop/SQLPage/tests/end-to-end/official-site.spec.ts:357:3
Error Context: test-results/official-site-no-console-errors-on-card-page-chromium/error-context.md
1 failed
[chromium] › official-site.spec.ts:356:5 › no console errors on card page ──────────────────────
21 passed (7.5s)
Serving HTML report at http://localhost:9323. Press Ctrl+C to quit. |
|
Hello and thank you for the PRs ! I see a lot of different things. Are these issues you encountered ? What do you use SQLPage for ? |
lovasoa
left a comment
lovasoa
left a comment
There was a problem hiding this comment.
This is useful as is, thank you !
But the warnings are bound to reappear if we do not add a CI gate...
|
thank you for the response!!
I am using it to render host and container level metrics stored in parquet format to power a dashboard. This solution seemed a lot more lightweight than Sentry and others.
Well my goal was to address #1362 but in the process of making those changes I ended up making a lot more changes to the codebase. In order to make it easier for maintainers to review I have tried to split them up into smaller commits stacked on top of eachother. #1367 is next
I agree, which is why one of the commits on the stack is to enable these tests in the CI |
2 participants
Motivation
biome check .reports 18 warnings on a clean checkout, so a warning introduced by a new change is easy to miss..claude/settings.local.jsonis checked too, and fails the run outright.Description
!importantfrom the declarations that do not need it.biome-ignorenaming the reason, in the three places that do: leaflet injects its stylesheet after ours, tabler's spacing utilities are themselves!important, and.markdown precaps the height a print rule has to override.mdastToDeltaandopen_modal_for_hash.biome-ignorefornoUnusedVariablesthat no longer matches anything.@ts-ignoreon a typedBuffer.fromcall, and give threeNumber.parseIntcalls their radix..claude/*.jsoninbiome.json, alongside the existing.zed/*.json.Testing
npm testchecks 36 files and reports nothing, where the base reports 18 warnings and 3 infos.no console errors on card pagefails here and on the unmodified base alike: the page loads avatars fromavatars.githubusercontent.com, which this machine cannot verify (ERR_CERT_AUTHORITY_INVALID).GitHub cannot base a pull request on a branch that lives in a fork, so all eight target
mainand each one carries the commits of those above it. Review and merge them in order: