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 5 commits intocloudfoundry:mainfrom
Conversation
TODO: I am unsure about this one. Needs more exploration
Some UAA do not return a Location header.
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.
| // Optional: bean requires V2 API; class is guarded by @RequiresV2Api | ||
| @Autowired(required = false) | ||
| private Mono<String> serviceBrokerId; |
There was a problem hiding this comment.
Isn't it surprising that the class pulls in the serviceBrokerId bean when it's full disabled? I would have expected that this bean is not loaded if the class doesn't execute.
| } | ||
|
|
||
| @Test | ||
| @RequiresV2Api |
There was a problem hiding this comment.
Only the info test can execute without the V2 API, and it doesn't bring much value. Let's annotate the whole class .
| private static final boolean RUN_V2_CLEANUP = isRunV2Tests(); | ||
|
|
||
| private static boolean isRunV2Tests() { | ||
| return !"true".equalsIgnoreCase(System.getenv("SKIP_V2_TESTS")); | ||
| } |
There was a problem hiding this comment.
Consider putting this in RequiresV2Api so the condition is shared.
| if (runV2Calls) { | ||
| apps = | ||
| apps.delayUntil( | ||
| application -> | ||
| removeApplicationServiceBindings( | ||
| cloudFoundryClient, application)); | ||
| } |
There was a problem hiding this comment.
That's the only v2 calls, which lists then deletes service bindings.
Can we port it to v3 instead of gating it with a boolean?
|
|
||
| @Bean(initMethod = "block") | ||
| @DependsOn("cloudFoundryCleaner") | ||
| @ConditionalOnProperty(name = "SKIP_V2_TESTS", havingValue = "false", matchIfMissing = true) |
There was a problem hiding this comment.
Let's centralize this property name, e.g. in RequiresV2Api.
| `TEST_PROXY_USERNAME` | _(Optional)_ The username for a proxy to route all requests through | ||
| `TEST_SKIPSSLVALIDATION` | _(Optional)_ Whether to skip SSL validation when connecting to the Cloud Foundry instance. Defaults to `false`. | ||
| `UAA_API_REQUEST_LIMIT` | _(Optional)_ If your UAA server does rate limiting and returns 429 errors, set this variable to the smallest limit configured there. Whether your server limits UAA calls is shown in the log, together with the location of the configuration file on the server. Defaults to `0` (no limit). | ||
| `SKIP_BROWSER_AUTH_TESTS` | _(Optional)_ Set to `true` to skip integration tests that require browser-based authentication (e.g., SSO tests). Useful when the Cloud Foundry instance does not support browser-based authentication or when running tests in a non-interactive environment. Defaults to `false`. |
There was a problem hiding this comment.
Is this really about "non-interactive" environments? Do we support interactive environments in our tests? Or is it about some UAA configuration that has browser-based auth?
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.