Skip to content

Raise unit test coverage to 100% of statements - #3918

Draft
Loyalsoldier with Copilot wants to merge 6 commits into
masterfrom
copilot/increase-unit-test-coverage
Draft

Raise unit test coverage to 100% of statements#3918
Loyalsoldier with Copilot wants to merge 6 commits into
masterfrom
copilot/increase-unit-test-coverage

Conversation

Copilot AI commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Statement coverage went from 43.7% to 100.0% (main 56.4% → 100.0%, cmd/datdump 0.0% → 100.0%). Test-only change; no production code touched. internal/dlc holds constants only, so it has no statements to cover.

Description

main_test.go

  • New table cases for uncovered parser branches: invalid keyword/attribute/affiliation, over-long domain, unknown inclusion field.
  • TestValidateChars for the validator paths unreachable from parseEntry (empty input).
  • loadData: comments/blank lines, type-less rules, affiliations (&list), inclusion/entry errors, missing file.
  • resolveList: unknown list, list that resolves to zero entries.
  • makeProtoList, loadTasks, writePlainList, and assembleDat across all/allowlist/denylist, including missing lists, empty allowlist, write failure, and marshal failure.
  • run() end-to-end (default task and --datprofile) plus its eight error paths; main() success path.

cmd/datdump/main_test.go (new)

  • loadGeosite, domain2Builder (all rule types plus an unknown one), exportSite, exportAll with the generated YAML asserted byte for byte, and run() for _all_, selected lists, and every failure branch.

Notes on approach

  • Failures are injected portably — a directory where an output file is expected, a file used as a parent directory — instead of permission bits, so the suite also passes as root.
  • main()'s os.Exit(1) branch is exercised by re-executing the test binary behind an env guard; the child's coverage counters are merged via the inherited GOCOVERDIR.
  • Dat assertions compare list and rule order, not just membership, so slices.Compact/slices.Sort on allowlist indexes and the reproducibility sort in run() are all load-bearing:
want := []datList{
	{name: "APPLE", rules: []string{"domain:apple.com", "domain:example.cn", ...}},
	{name: "CN", rules: []string{"domain:example.cn", ...}},
	...
}
  • Package-level flag variables and os.Args are restored via t.Cleanup; the suite is green under -race and -count=2 -shuffle=on.

Worth noting for reviewers: neither workflow runs go test, so these tests execute locally only unless a CI step is added.

Copilot AI and others added 3 commits August 5, 2026 14:27
Co-authored-by: Loyalsoldier <10487845+Loyalsoldier@users.noreply.github.com>
Co-authored-by: Loyalsoldier <10487845+Loyalsoldier@users.noreply.github.com>
Co-authored-by: Loyalsoldier <10487845+Loyalsoldier@users.noreply.github.com>
@Loyalsoldier

Copy link
Copy Markdown
Collaborator

@copilot Review this PR, find mistakes and potential bugs, fix them if any.

Copilot AI and others added 2 commits August 5, 2026 14:57
Co-authored-by: Loyalsoldier <10487845+Loyalsoldier@users.noreply.github.com>
Co-authored-by: Loyalsoldier <10487845+Loyalsoldier@users.noreply.github.com>

Copilot AI commented Aug 5, 2026

Copy link
Copy Markdown
Contributor Author

@copilot Review this PR, find mistakes and potential bugs, fix them if any.

Fixed the subprocess temp-directory leak in 3347d40b and added go test ./... to the PR workflow in 4ddbebf4. Race/shuffle tests still report 100% statement coverage, go vet passes, and CodeQL found no alerts. The latest Actions run has zero jobs and is awaiting workflow approval (action_required), not failing.

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.

2 participants