Skip to content

test(e2e): add Podman provider e2e test parity with Docker#284

Merged
skevetter merged 2 commits into
mainfrom
skevetter/devsy-100-podman-e2e-parity
May 14, 2026
Merged

test(e2e): add Podman provider e2e test parity with Docker#284
skevetter merged 2 commits into
mainfrom
skevetter/devsy-100-podman-e2e-parity

Conversation

@skevetter

@skevetter skevetter commented May 14, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds 15 Docker e2e test scenarios to the Podman provider, for both rootless and rootful contexts (30 new test cases total, bringing Podman from 14 to 44 test specs)
  • Tests cover configuration (variable substitution, defaults, remoteEnv null, extra devcontainer merge/override, multi devcontainer selection), lifecycle commands (postStartCommand restart, waitFor deferred postCreate, postAttach nonblocking/every-attach, initializeCommand parallel, secrets-file injection), and features (mounts, custom image, custom image skip build)
  • All tests use existing Docker testdata fixtures and framework-only helpers — no Docker SDK dependencies
  • Docker tests that require docker.DockerHelper.Inspect with Docker SDK types (existing running container, security options, custom workspace mount, workspace mount consistency, overrideCommand default, id-label) are intentionally omitted from Podman as they cannot be implemented without the prohibited imports

Closes DEVSY-100

Summary by CodeRabbit

  • Tests
    • Added comprehensive e2e test coverage for Podman provider, validating lifecycle command execution, environment variable handling, secrets injection, configuration merging, and custom image support for both rootless and rootful modes.

Review Change Stack

Add 15 missing Docker e2e test scenarios to both rootless and rootful
Podman contexts, bringing Podman test coverage to parity with Docker
for all tests that don't require Docker SDK types.
@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR expands e2e test coverage for the Podman provider by adding lifecycle command and configuration/feature validation tests for both rootless and rootful Podman modes, including assertions for command execution order, deferred execution, IDE accessibility timing, secrets injection, variable substitution, config merging, and custom image handling.

Changes

Podman e2e test coverage

Layer / File(s) Summary
Test imports
e2e/tests/up/provider_podman.go (lines 7–10)
Import list updated to add path, path/filepath, and time packages supporting filesystem path construction, file reading, and timed polling assertions in the new test specs.
Rootless Podman lifecycle and configuration tests
e2e/tests/up/provider_podman.go (lines 110–638)
Rootless Podman test suite validates postStartCommand restart behavior, deferred postCreateCommand execution with waitFor and environment placeholder expansion, IDE accessibility timing versus postAttachCommand completion, repeated postAttachCommand execution on every attach, initializeCommand object-syntax execution via output file reading, secrets-file injection into lifecycle environment, variable substitution with defaults, extra devcontainer config merging and overriding, multiple devcontainer selection, volume mounts, custom devcontainer images, and build skipping.
Rootful Podman lifecycle and configuration tests
e2e/tests/up/provider_podman.go (lines 744–1272)
Rootful Podman test suite mirrors the rootless coverage with identical lifecycle command and configuration assertions: postStartCommand restart, deferred postCreateCommand with waitFor, postAttachCommand execution timing and repetition, initializeCommand object syntax, secrets-file injection, variable substitution, extra devcontainer config merging, devcontainer selection, volume mounting, custom image usage, and build skipping.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • devsy-org/devsy#280: Both PRs expand and reorganize e2e/tests/up/provider_podman.go, adding and structuring new rootless and rootful Podman e2e scenarios around lifecycle commands and related assertions.
  • devsy-org/devsy#278: The CI workflow matrix update directly exercises the newly added rootless and rootful Podman test cases by installing and running up-provider-podman in both modes.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding Podman provider e2e tests to achieve parity with Docker coverage, which matches the changeset's addition of 30 new Podman test cases.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@netlify

netlify Bot commented May 14, 2026

Copy link
Copy Markdown

Deploy Preview for devsydev canceled.

Name Link
🔨 Latest commit 8ad9b91
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6a05b133cf87e10008346feb

@skevetter
skevetter enabled auto-merge (squash) May 14, 2026 11:10
The docker-remote-env-null testdata uses a Dockerfile build that
fails with Podman's buildah backend in CI. Remove the test from
both rootless and rootful contexts until the build issue is resolved.

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@e2e/tests/up/provider_podman.go`:
- Around line 124-126: The current line-count logic uses lines :=
strings.Count(strings.TrimSpace(out), "\n") + 1 which returns 1 for an empty
string and can false-pass; update the assertion to first check for an empty
trimmed out (e.g., if strings.TrimSpace(out) == "" then treat lines as 0 or fail
the test), otherwise compute lines by splitting on "\n" and counting non-empty
entries, and then assert the expected count; modify the checks around the
variable out (the lines calculation and the gomega.Expect call) in
provider_podman.go so empty logs do not report as one line.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 44b5b47d-286b-4a57-8919-d1abb1473595

📥 Commits

Reviewing files that changed from the base of the PR and between 1eaf03d and 8ad9b91.

📒 Files selected for processing (1)
  • e2e/tests/up/provider_podman.go

Comment on lines +124 to +126
lines := strings.Count(strings.TrimSpace(out), "\n") + 1
gomega.Expect(lines).To(gomega.Equal(1),
"postStartCommand should have run once after initial up")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Line counting can false-pass when the log is empty.

At Line 124 and Line 758, strings.Count(strings.TrimSpace(out), "\n") + 1 evaluates to 1 for out == "", so the assertion can pass even if postStartCommand never wrote anything.

✅ Suggested fix
- out, err := f.DevsySSH(ctx, tempDir, "cat $HOME/post-start-count.log")
- framework.ExpectNoError(err)
- lines := strings.Count(strings.TrimSpace(out), "\n") + 1
+ out, err := f.DevsySSH(ctx, tempDir, "cat $HOME/post-start-count.log")
+ framework.ExpectNoError(err)
+ trimmed := strings.TrimSpace(out)
+ gomega.Expect(trimmed).NotTo(gomega.BeEmpty())
+ lines := len(strings.Split(trimmed, "\n"))
  gomega.Expect(lines).To(gomega.Equal(1),
      "postStartCommand should have run once after initial up")
...
- out, err = f.DevsySSH(ctx, tempDir, "cat $HOME/post-start-count.log")
- framework.ExpectNoError(err)
- lines = strings.Count(strings.TrimSpace(out), "\n") + 1
+ out, err = f.DevsySSH(ctx, tempDir, "cat $HOME/post-start-count.log")
+ framework.ExpectNoError(err)
+ trimmed = strings.TrimSpace(out)
+ gomega.Expect(trimmed).NotTo(gomega.BeEmpty())
+ lines = len(strings.Split(trimmed, "\n"))
  gomega.Expect(lines).To(gomega.Equal(2),
      "postStartCommand should have run again after restart")

Also applies to: 136-138, 758-760, 770-772

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@e2e/tests/up/provider_podman.go` around lines 124 - 126, The current
line-count logic uses lines := strings.Count(strings.TrimSpace(out), "\n") + 1
which returns 1 for an empty string and can false-pass; update the assertion to
first check for an empty trimmed out (e.g., if strings.TrimSpace(out) == "" then
treat lines as 0 or fail the test), otherwise compute lines by splitting on "\n"
and counting non-empty entries, and then assert the expected count; modify the
checks around the variable out (the lines calculation and the gomega.Expect
call) in provider_podman.go so empty logs do not report as one line.

@skevetter
skevetter merged commit 49ed3fe into main May 14, 2026
57 checks passed
@skevetter
skevetter deleted the skevetter/devsy-100-podman-e2e-parity branch May 14, 2026 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant