Skip to content

test(e2e): add end-to-end coverage for kmeshctl log - #1942

Open
shreyaabaranwal wants to merge 2 commits into
kmesh-net:mainfrom
shreyaabaranwal:test/kmeshctl-log-e2e
Open

shreyaabaranwal wants to merge 2 commits into
kmesh-net:mainfrom
shreyaabaranwal:test/kmeshctl-log-e2e

Conversation

@shreyaabaranwal

@shreyaabaranwal shreyaabaranwal commented Aug 26, 2026

Copy link
Copy Markdown

What this does

Adds e2e coverage for kmeshctl log (list loggers, get a logger's level,
set a logger's level, and reject an invalid --set value) , currently
the only test coverage for this command is a narrow unit test for
URL-building in ctl/log/log_test.go. No production code changes.

Context

Follows the same focused, single-command pattern proposed in #1819 for
kmeshctl dump. An earlier broader attempt (#1316) covering multiple
kmeshctl subcommands stalled on merge-commit/rebase issues rather than
the code itself; this PR incorporates the review feedback given there by
@YaoZengzeng: gofmt-clean, asserting on actual output content rather
than printing it, polling via retry.UntilSuccess instead of
time.Sleep, and a single parameterized helper instead of
per-subcommand duplicates. The "set" subtest restores the logger's
original level via t.Cleanup so it doesn't leak state into other
tests.

Testing

  • gofmt -l is clean.
  • go build -tags integ ./test/e2e/... and go vet -tags integ ./test/e2e/... pass.
  • I cross-checked every assertion in the test against the real
    implementation (ctl/log/log.go and pkg/status/status_server.go)
    and against the existing kmeshctl log --set assertions already made
    by test/e2e/run_test.sh itself.
  • I was not able to complete a live run against a kind cluster in my own
    environment before opening this PR (an environment issue on my end,
    unrelated to this code) and will do so before this is merged . I'll
    update this PR with confirmation once that run passes, and am happy to
    hold off on merge until then.

Special notes for your reviewer

This test was drafted with the assistance of Claude (Anthropic), based
on patterns already present in this repo's e2e suite
(test/e2e/restart_test.go, test/e2e/main_test.go) and incorporating
the review feedback previously given on PR #1316. As noted above, I have
not yet completed a live kind-cluster run of this test . I'll confirm
that separately before merge. I'm happy to walk through any part of the
implementation.

Signed-off-by: shreyaabaranwal <shreyabaranwal229@gmail.com>
@kmesh-bot

Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign yaozengzeng for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kmesh-bot

Copy link
Copy Markdown
Collaborator

Welcome @shreyaabaranwal! It looks like this is your first PR to kmesh-net/kmesh 🎉

@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 39.58%. Comparing base (c6357aa) to head (66924fd).
⚠️ Report is 14 commits behind head on main.
see 2 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ec501a9...66924fd. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@shreyaabaranwal

Copy link
Copy Markdown
Author

Update: I wasn't able to complete a live kind-cluster run in my local
sandbox (hit a few environment-specific issues there , inotify limits,
Docker userns-remap, disk space , all unrelated to this test's code),
but this PR's own CI has now run the full e2e suite including
TestKmeshctlLog against real clusters, and all E2E Test and E2E IPv6
Test checks (istio 1.26/1.27/1.28) are passing. Considering that the
live verification I mentioned in the PR description , happy to answer
any questions on the implementation.

@kmesh-bot

Copy link
Copy Markdown
Collaborator

Adding label do-not-merge/contains-merge-commits because PR contains merge commits, which are not allowed in this repository.
Use git rebase to reapply your commits on top of the target branch. Detailed instructions for doing so can be found here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants