chore: upgrade GitHub Actions + close e2e/smoke test coverage gaps#49
chore: upgrade GitHub Actions + close e2e/smoke test coverage gaps#49PenguinzTech merged 3 commits intov2.1.xfrom
Conversation
Actions upgrades: - actions/checkout v5→v6, actions/cache v4→v5, actions/setup-go v5→v6 - docker/setup-qemu-action v3→v4, docker/setup-buildx-action v3→v4 - docker/build-push-action v6→v7, docker/login-action v3→v4 - docker/metadata-action → v6, linear-b/gitstream-github-action → v2 Test coverage gaps closed (per standards audit): - /ready readiness endpoint tests in DNS server and web console smoke suites - API filter/pagination param tests: /api/v1/queries (page, limit, empty page), /api/v1/domains (active filter), /api/v1/logs, /api/v1/ioc/feeds invalid-ID 404 - Playwright: one-time auth fixture + protected-route page-load tests for all 7 web console routes (index, tokens, domains, permissions, blacklist, certs, logs) - Playwright: form modal validation-error tests (new token, new domain) - CI cleanup step (if: always()) for /tmp/playwright-squawk artifacts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Reviewer's GuideThis PR upgrades all GitHub Actions to Node.js 24–compatible versions across workflows and expands smoke/e2e coverage for DNS server and web console readiness, API filters/pagination, and auth-protected UI flows, including Playwright artifact cleanup in CI. Sequence diagram for CI build-and-test workflow with Playwright cleanupsequenceDiagram
participant Dev as Developer
participant GH as GitHub_Actions
participant Job as build-and-test_job
participant PW as Playwright_tests
participant FS as CI_FileSystem
Dev->>GH: Push commit / open PR
GH->>Job: Start build-and-test workflow
Job->>Job: Run unit tests and smoke tests
Job->>PW: Start Playwright E2E tests (dns-webui)
PW->>FS: Write artifacts to /tmp/playwright-squawk
PW-->>Job: Report test result (pass/fail)
Job->>Job: Execute Clean_up_Playwright_artifacts (if always)
Job->>FS: rm -rf /tmp/playwright-squawk
FS-->>Job: Artifacts directory removed
Job-->>GH: Report overall job status
GH-->>Dev: Show CI result on PR
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
dns-webui.spec.ts, consider applying theservicesRunning()skip at thedescribelevel (e.g., viatest.describe.configureor abeforeEachthat callstest.skip) to avoid repeating the same skip logic in every test body. - The Playwright form modal tests rely on broad CSS selectors for the submit button and error messages; if possible, introducing stable
data-testidattributes and targeting those would make these tests less brittle against UI or styling changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `dns-webui.spec.ts`, consider applying the `servicesRunning()` skip at the `describe` level (e.g., via `test.describe.configure` or a `beforeEach` that calls `test.skip`) to avoid repeating the same skip logic in every test body.
- The Playwright form modal tests rely on broad CSS selectors for the submit button and error messages; if possible, introducing stable `data-testid` attributes and targeting those would make these tests less brittle against UI or styling changes.
## Individual Comments
### Comment 1
<location path="tests/smoke/test_web_console_api.py" line_range="195-202" />
<code_context>
+ assert "queries" in data
+ assert len(data["queries"]) <= 10
+
+ def test_queries_empty_far_page(self, authenticated_client):
+ """GET /api/v1/queries with a far-out page returns empty list, not error"""
+ response = authenticated_client.get("/api/v1/queries", params={"page": 99999, "limit": 10})
+
+ assert response.status_code == 200
+ data = response.json()
+ items = data.get("queries", data.get("items", data.get("data", [])))
+ assert isinstance(items, list)
+
+ def test_domains_filter_active(self, authenticated_client):
</code_context>
<issue_to_address>
**issue (testing):** Test name/docstring say "returns empty list" but the assertions only check list type, not emptiness.
Right now the test would still pass if the far-out page returned a non-empty list, which conflicts with the described behavior and reduces its value as a regression test.
If the contract is truly “empty list for far pages”, please assert that explicitly, e.g.:
```python
items = data.get("queries", data.get("items", data.get("data", [])))
assert isinstance(items, list)
assert len(items) == 0
```
Otherwise, if some items are acceptable, consider relaxing the docstring and instead asserting a more accurate invariant (e.g. no error and correct payload shape).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| def test_queries_empty_far_page(self, authenticated_client): | ||
| """GET /api/v1/queries with a far-out page returns empty list, not error""" | ||
| response = authenticated_client.get("/api/v1/queries", params={"page": 99999, "limit": 10}) | ||
|
|
||
| assert response.status_code == 200 | ||
| data = response.json() | ||
| items = data.get("queries", data.get("items", data.get("data", []))) | ||
| assert isinstance(items, list) |
There was a problem hiding this comment.
issue (testing): Test name/docstring say "returns empty list" but the assertions only check list type, not emptiness.
Right now the test would still pass if the far-out page returned a non-empty list, which conflicts with the described behavior and reduces its value as a regression test.
If the contract is truly “empty list for far pages”, please assert that explicitly, e.g.:
items = data.get("queries", data.get("items", data.get("data", [])))
assert isinstance(items, list)
assert len(items) == 0Otherwise, if some items are acceptable, consider relaxing the docstring and instead asserting a more accurate invariant (e.g. no error and correct payload shape).
Replace nginx Ingress with Gateway API HTTPRoute targeting the shared Gateway in the gateway namespace. Part of dal2-beta nginx-ingress EOL. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No dependency changes detected. Learn more about Socket for GitHub. 👍 No dependency changes detected in pull request |
gosec@latest resolves to v2.25.0 which requires Go >= 1.25.0. Pin to v2.22.11 (go 1.24.0 minimum) to match our Go 1.24.2 toolchain. Also fixes the @latest ban from dependency pinning standards. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
testing.md/testing-react.md/testing-python.mdstandardsTest coverage added
tests/smoke/test_dns_server_api.pytest_ready_endpoint—/readyreadiness checktests/smoke/test_web_console_api.py/readytest;TestQueryFilterParams— pagination params, empty far-page,activedomain filter, logs pagination, invalid IOC feed ID → 404tests/e2e/dns-webui.spec.ts.github/workflows/build.ymlif: always())Standards compliance
testing.md—/readysmoke test, API filter coverage, PlaywrightoutputDircleanuptesting-react.md— auth-protected page loads, form modal open/validation,CI: add cleanup step with if: always()Test plan
pytest tests/smoke/test_dns_server_api.py::TestDNSServerHealth::test_ready_endpoint -vpytest tests/smoke/test_web_console_api.py::TestWebConsoleHealth -vpytest tests/smoke/test_web_console_api.py::TestQueryFilterParams -vcd tests/e2e && CI_SERVICES_RUNNING=true npx playwright test dns-webui(with services running)🤖 Generated with Claude Code
Summary by Sourcery
Upgrade CI workflows to current GitHub Actions versions and expand readiness, filtering, and UI smoke coverage for the DNS server and web console.
New Features:
Enhancements:
Build:
CI: