Skip to content

fix(integration): add missing container detection test file and fixtures - #1404

Merged
stokpop merged 2 commits into
cloudfoundry:mainfrom
stokpop:fix/1361-missing-container-detection-test
Aug 28, 2026
Merged

fix(integration): add missing container detection test file and fixtures#1404
stokpop merged 2 commits into
cloudfoundry:mainfrom
stokpop:fix/1361-missing-container-detection-test

Conversation

@stokpop

@stokpop stokpop commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Summary

PR #1357 committed the ContainerDetection suite registration in
init_test.go but not the test file or fixtures, breaking CI:

src/integration/init_test.go:103:30: undefined: testContainerDetectionErrors
  • Add container_detection_test.go and 9 testdata fixture directories
  • Empty directories built at runtime by stageFixture (git can't track them)
  • Tighten two tests that passed unconditionally to assert on "Detected container:" log
  • Align cleanup with established pattern (KeepFailedContainers, failed-test logging)

Test plan

  • go vet passes with and fails without the file
  • 10/10 pass from clean checkout (git archive) against real docker staging (~14 min)
  • No temp/docker leaks after run

PR cloudfoundry#1357 added the ContainerDetection suite registration in init_test.go
but not container_detection_test.go or its testdata fixtures, breaking
the build on clean checkout:

  src/integration/init_test.go:103:30: undefined: testContainerDetectionErrors

Add the test file and fixtures. Empty directories (BOOT-INF/classes,
BOOT-INF/lib, empty app dir) are built at runtime by stageFixture
since git cannot track empty directories. Two tests that passed
unconditionally now assert on "Detected container:" log output.

Verified: 10/10 pass from clean checkout against real docker staging.

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.

Pull request overview

This PR restores the missing ContainerDetection integration suite that was registered in init_test.go (PR #1357) by adding the actual test implementation and the required fixture directories/files under src/integration/testdata/containers, unblocking CI.

Changes:

  • Add container_detection_test.go implementing the testContainerDetectionErrors suite (including runtime creation of empty directories via stageFixture).
  • Add container-detection fixtures covering “no container” scenarios and Java Main / Spring Boot detection edge cases.
  • Add minimal manifest/source/class marker files used by the new integration tests.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/integration/container_detection_test.go Adds the ContainerDetection integration suite and fixture staging helper.
src/integration/testdata/containers/thin_jar_with_main_class/META-INF/MANIFEST.MF Fixture manifest with Main-Class to exercise Java Main detection.
src/integration/testdata/containers/thin_jar_with_main_class/com/example/Application.class Stub class file for the thin-jar-with-main-class fixture.
src/integration/testdata/containers/no_container_thin_jar/META-INF/MANIFEST.MF Fixture manifest intentionally missing Main-Class to ensure container detection fails.
src/integration/testdata/containers/no_container_thin_jar_extracted/META-INF/maven/com.example/myapp/pom.properties Extracted-jar-style Maven metadata fixture.
src/integration/testdata/containers/no_container_thin_jar_extracted/META-INF/MANIFEST.MF Extracted thin-jar manifest without Main-Class for failure coverage.
src/integration/testdata/containers/no_container_thin_jar_extracted/com/example/Application.class Stub class file for extracted thin-jar fixture.
src/integration/testdata/containers/no_container_text_file/README.txt Non-Java-only fixture to validate detect rejection.
src/integration/testdata/containers/no_container_target_classes/META-INF/maven/com.example/myapp/pom.properties “Compiled classes only” fixture Maven metadata.
src/integration/testdata/containers/no_container_target_classes/com/example/Application.class Stub class file for “compiled classes only” fixture.
src/integration/testdata/containers/no_container_maven_source/src/main/java/com/example/Application.java Maven source-only fixture (no compiled artifact) to validate failure path.
src/integration/testdata/containers/no_container_maven_source/pom.xml Maven source-only fixture POM.
src/integration/testdata/containers/no_container_boot_inf_no_markers/META-INF/MANIFEST.MF BOOT-INF-present fixture manifest intentionally missing Spring Boot markers.
src/integration/testdata/containers/no_container_boot_inf_empty_lib/META-INF/MANIFEST.MF Spring Boot markers fixture used with runtime-created empty BOOT-INF/lib.

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

Comment thread src/integration/container_detection_test.go

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.

Pull request overview

Copilot reviewed 14 out of 15 changed files in this pull request and generated no new comments.

@kiril-keranov kiril-keranov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks fine

@stokpop
stokpop merged commit 9046df0 into cloudfoundry:main Aug 28, 2026
1 check passed
@stokpop

stokpop commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

@kiril-keranov these tests now run, but I see another SB4 with JBP5 failure with automatic jdbc detection from cf env not working: not sure why this test also was not failing before locally... I am investigating if this is a real JPB + SB autodetection bug

@stokpop

stokpop commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

@kiril-keranov took some time to figure it out, but I think there is an actual issue here, see: #1406 (and link to pivotal-cf/java-cfenv)... we have dep on java-cfenv fix to get this failing test working... what to do in the mean time?

Skip the failing test till fix is ready?

@kiril-keranov

kiril-keranov commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

@kiril-keranov took some time to figure it out, but I think there is an actual issue here, see: #1406 (and link to pivotal-cf/java-cfenv)... we have dep on java-cfenv fix to get this failing test working... what to do in the mean time?

Skip the failing test till fix is ready?

@stokpop Ok, let's ignore the test currently. I just wanted to trigger a new release with the latest versions added that address some security findings. Do the bump of java-cfenv triggered this issue?

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.

3 participants