Add scoped-credential integration tests#590
Conversation
23384cb to
8aad8e5
Compare
| // Enable when the auth-file property is set (file-based tests) or when scoped credentials are | ||
| // supplied via the environment (scoped user/team/shared-link tests). Tests whose credentials | ||
| // are not present skip themselves. | ||
| enabled = project.hasProperty(authInfoPropertyName) || | ||
| System.getenv('SCOPED_USER_REFRESH_TOKEN') != null || | ||
| System.getenv('SCOPED_TEAM_REFRESH_TOKEN') != null | ||
|
|
||
| // Run against the same compiled classes and classpath as the unit `test` task. | ||
| testClassesDirs = sourceSets.test.output.classesDirs | ||
| classpath = sourceSets.test.runtimeClasspath |
There was a problem hiding this comment.
I'm not sure this is correct? I don't see anywhere that disables tests based on credentials not being there. If you don't have the authInfoPropertyName then when any test tries to load those credentials it will fail
But this keeps all tests enabled if you provide just a SCOPED_TEAM_REFRESH_TOKEN which that could also fail if you don't set the other required environment variables (SCOPED_TEAM_CLIENT_ID and SCOPED_TEAM_CLIENT_SECRET)
Plus i'm not sure what the changes are for the testClassesDirs and classpath that were needed as part of this
There was a problem hiding this comment.
Gradle explicitly documents that, starting with 9.0, only the built-in test task is automatically configured. Custom Test tasks must configure both testClassesDirs and classpath. Gradle documentation
| -d client_id=$APP_KEY \ | ||
| -d client_secret=$APP_SECRET > auth_output |
There was a problem hiding this comment.
We had the app secret before but didn't need it as part of our token refresh, is that now needed? This also doesn't refresh the tokens for the team or user secrets other, is that needed?
There was a problem hiding this comment.
introduced new special tests for PKCE.
| public static String valueFromEnvOrSkip(String name) { | ||
| String value = System.getenv(name); | ||
| if (value == null || value.isEmpty()) { | ||
| throw new SkipException( | ||
| "Environment variable \"" + name + "\" is not set; skipping test that requires " + | ||
| "shared SDK integration credentials."); | ||
| } | ||
| return value; | ||
| } |
There was a problem hiding this comment.
We cannot read environment variables that are set on the system. Per the docs:
By default, Gradle does not automatically pass all terminal environment variables to the forked test process. You need to configure your integrationTest task to inherit them.
This is why we kick off tests we use -Pcom.dropbox.test.authInfoFile=../auth_output in our gradle command, which passes the variable to test process.
Integration tests don't run on PRs, only when we land, so you'll need to run them locally and confirm
There was a problem hiding this comment.
Confirmed with a temporary PR integration run: https://github.com/dropbox/dropbox-sdk-java/actions/runs/29540327341
The integration job passed for OkHttpRequestor, OkHttp3Requestor, and StandardHttpRequestor. ScopedUserClientV2IT, SharedLinkV2IT, and TeamClientV2IT ran successfully in all three suites while reading the scoped credentials through System.getenv().
This confirms that the environment variables configured on the workflow step are inherited by the forked Gradle test process. authInfoFile uses a separate mechanism: -Pcom.dropbox.test.authInfoFile is a Gradle project property that core/build.gradle explicitly forwards as a JVM system property.
afeb032 to
a59fd23
Compare
2 participants
Summary
Adds integration coverage for scoped user and team clients authenticated with refresh tokens, plus shared-link resolution.
Changes
integrationTesttask with the test source set and runtime classpath.Validation
git diff --check