test: cover import, avatars, and chart theme; restore coverage gates - #107
Merged
Conversation
…gates Closes #95. - import.ts had no tests at all. Covers CSV, ZIP, non-CSV skip, and per-entry failure isolation. The ZIP fixture is prebuilt because fflate's zipSync can't produce a valid archive under jsdom — its Uint8Array check fails across realms and it walks the byte array as a nested directory. The read path is unaffected, which is what the app actually uses on import. - The bot avatar code was untested because this jsdom setup exposes no localStorage, so cache hydration and persistence silently no-op'd. Installing an in-memory Storage exercises them, along with request dedupe, the concurrency queue, the 10-lookup batch cap, and failure handling. - buildGitHubChartTheme now covers both the CSS-variable path and the bundled-palette fallback. Thresholds go back to the 80/70/80/65 the issue asked for. Actual is 88.2 statements / 73.2 branches / 88.1 functions / 90.8 lines. Also corrects the zip.ts coverage-exclusion comment, which blamed File.arrayBuffer(); that works fine here, zipSync is the problem. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Org-level Copilot seat exports have no Organization column, but the seat activity schema defaults to grouping by it. The fallback couldn't help because the fallback was the broken column, so the chart came up empty. Validate each candidate in turn — current column, schema default, then the report's primary dimension — instead of trusting the default. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Switching pages in the sidebar reset the grouping to the schema default without checking the active report actually has that column. On an org-level seat export, which has no Organization column, that put the page straight back into the empty-chart state the upload path had just avoided. Resolve against the report being switched to. Verified against a five-column org-level export: the chart now renders per-login seats instead of coming up blank. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #95.
Stacked on #106.
import.tshad zero coverage and the bot-avatar code informatters.tswas effectively untestable, because this jsdom setup exposes nolocalStorage— so cache hydration and persistence silently no-op'd inside theirtry/catch.What's covered now
import.tsformatters.tschart-theme.tsThresholds go back to the 80/70/80/65 the issue asked for. Actual is 88.2 statements / 73.2 branches / 88.1 functions / 90.8 lines, so there's real headroom rather than a gate pinned to the current number.
Two things worth knowing
The ZIP fixture is a prebuilt base64 blob rather than something the test builds.
fflate'szipSynccan't produce a valid archive under jsdom: itsinstanceof Uint8Arraycheck fails across realms, so it walks the byte array as a nested directory and emits entries likea.csv/0/,a.csv/1/. The same call in plain Node produces a correct 117-byte archive.unzipSyncis unaffected, and that's the only half the app uses, so this is a test-environment artifact and not a product bug.The
zip.tscoverage-exclusion comment blamedFile.arrayBuffer()not existing in jsdom. It does exist and works — I checked, it returned the bytes.zipSyncis the actual blocker, so the comment now says so.285 tests pass, 0 lint errors, 0 type errors.