Add scoped-credential integration tests#590
Conversation
AndreyVMarkelov
requested review from
devPalacio and
rharter
and removed request for
rharter
Add integration coverage for scoped user and team refresh tokens and shared links using CI-provided credentials. Configure the integrationTest task with the test source set and runtime classpath, and update CI to generate the auth file used by existing tests.
AndreyVMarkelov
force-pushed
the
integration-test-scoped-creds
branch
from
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.
Only if introduce new special tests for PKCE. Removed.
| 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
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