Skip to content

test(e2e): Add e2e coverage for kraft system - #2929

Merged
craciunoiuc merged 1 commit into
unikraft:stagingfrom
srinivasr:test/e2e-kraft-system
Sep 29, 2026
Merged

craciunoiuc merged 1 commit into
unikraft:stagingfrom
srinivasr:test/e2e-kraft-system

Conversation

@srinivasr

@srinivasr srinivasr commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Prerequisite checklist

Description of changes

Ref: #370

Adds e2e tests for kraft system set, list and unset in test/e2e/cli/system_test.go (21 specs).

set covers single and multiple keys, overwrites, and keeps existing keys. list checks the key=value output line by line since map order is random. unset checks the key is gone from the file and from list, other keys survive, and a removed key does not come back after a later set - that guards the bug from #2901, fixed in #2907. Also error cases (missing =, unknown keys, bad bool value, wrong arg counts) and --help output for the three subcommands.

Testing

ginkgo --focus "kraft system" test/e2e/cli/ - 21 passed, 0 failed, serial and parallel/random. gofumpt, vet and make lint clean.

Note: system unset on a scalar like log.level resets it to "" instead of the default. Left out of the assertions - happy to open an issue if that's unintended.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The list/unset coverage can pass without verifying expected list output.

Review effort: Lite
Findings: None

What changed in this PR

Adds 21 end-to-end tests for kraft system set, list, and unset commands.

Changes:

  • Tests persistence, overwrites, removal, validation errors, and help output.
  • Covers regression behavior for deleted keys.
  • The list/unset test should assert known output before checking absence.
File Description
test/​e2e/​cli/​system_test.go Adds 21 CLI end-to-end specifications.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@craciunoiuc craciunoiuc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey

could you remove these comments?

Otherwise pretty good to go

Comment thread test/e2e/cli/system_test.go Outdated
Comment thread test/e2e/cli/system_test.go Outdated
Signed-off-by: srinivasr <sriniv4sreddy@gmail.com>
@srinivasr
srinivasr force-pushed the test/e2e-kraft-system branch from 0855fa3 to eb38a75 Compare September 29, 2026 14:23
@srinivasr

Copy link
Copy Markdown
Contributor Author

Hey

could you remove these comments?

Otherwise pretty good to go

done, removed both comment blocks, no other changes

@craciunoiuc craciunoiuc left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good here. Thanks!

Reviewed-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.com>
Approved-by: Cezar Craciunoiu <cezar.craciunoiu@unikraft.com>

@craciunoiuc
craciunoiuc merged commit 22e305d into unikraft:staging Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 🚀 Done

Development

Successfully merging this pull request may close these issues.

3 participants