Support skipping integration tests for unavailable CF features via environment variables#1346
Support skipping integration tests for unavailable CF features via environment variables#1346jorbaum wants to merge 11 commits intocloudfoundry:5.x.xfrom
Conversation
jorbaum
changed the title
jorbaum
force-pushed
the
fix-integration-tests
branch
from
0c5a424 to
5ad1072
Compare
There was a problem hiding this comment.
- Really clear annotations for execution conditions, I like the approach.
- A few questions and remarks throughout the PR.
- Please target
5.x.x
My biggest issue is that half of the classes in the v3 package are annotated with @RequiresV2Api. It seems v2 is required because we push applications (service broker, most notably) in the config with the v2 API, and those tests needs application pushed.
Could you please consider updating the core integration test config to use v3 apis for pushing apps instead?
To have examples of this, look at the ApplicationsTest in the v3 package.
|
One big reason for why I wanted to create a draf PR initially (which, apparently I did not :P ) is that I only tested the new annotations in the following combination: SKIP_V2_TESTS=false \
SKIP_TCP_ROUTING_TESTS=true \
SKIP_BROWSER_AUTH_TESTS=true \
SKIP_METRIC_REGISTRAR_TESTS=trueand SKIP_V2_TESTS=true \
SKIP_TCP_ROUTING_TESTS=true \
SKIP_BROWSER_AUTH_TESTS=true \
SKIP_METRIC_REGISTRAR_TESTS=trueThis is because my test environment does not support tcp routing, browser-based auth or cf marketplace. Do you think this is fine? |
|
I think it's fine. I'll test locally before merging, with and without the flags. please also target 5.x.x, we'll merge this back into main later. |
TODO: I am unsure about this one. Needs more exploration # Conflicts: # README.md # integration-test/src/test/java/org/cloudfoundry/operations/StacksTest.java
Some UAA do not return a Location header.
Prevent the serviceBrokerId bean from being created when SKIP_V2_TESTS=true, rather than relying on @Autowired(required=false) in each test class. This avoids the bean's initMethod="block" eagerly calling the V2 API at context startup even when all V2 tests are disabled.
Extract the "SKIP_V2_TESTS" string literal into a public constant RequiresV2Api.SKIP_V2_TESTS_ENV and reference it from IntegrationTestConfiguration and CloudFoundryCleaner.
The info() test alone doesn't bring enough value to justify keeping the class partially enabled without V2.
Replace V2 applicationsV2().listServiceBindings / removeServiceBinding calls with V3 serviceBindingsV3().list / delete in the cleaner's application cleanup path, eliminating the runV2Calls boolean parameter.
Reword to explain the env var is for when UAA doesn't have browser-based auth configured, rather than referencing "non-interactive environments".
Move the SKIP_V2_TESTS check into a reusable static method in V2ApiCondition.
jorbaum
force-pushed
the
fix-integration-tests
branch
from
5ad1072 to
25cb827
Compare
I have now addressed all of the comments except for this one. Moving the test config to use v3 seems like a bigger endeavor. I did not try to tackle it so far. Also I did not manage to rerun the integration tests again. Feel free to take a look at my changes already though. |
2 participants
On my CF test environment quite a few integration tests needed features that my environment did not support.
I added a bunch of environment variables so those tests can be disabled at a global level.
I am not sure about this approach yet. Especially regarding disabling V2 CAPI as it touches quite a lot of places. Any feedback welcome!
AI tools used: Claude Code and GitHub Copilot (Opus 4.6) assisted me during development. I reviewed the result.