Conversation
…I Test users. Fix issues with Token Migration of non-current user.
… element clicks. Change timeout values based on if running in CI.
Generated by 🚫 Danger |
| sidCookieName = callbackUrlParams.get(SID_COOKIE_NAME); | ||
| parentSid = callbackUrlParams.get(PARENT_SID); | ||
| tokenFormat = callbackUrlParams.get(TOKEN_FORMAT); | ||
| tokenFormat = callbackUrlParams.getOrDefault(TOKEN_FORMAT, ""); |
There was a problem hiding this comment.
This was a bug that came out of UI testing. The tokenFormat for opaque tokens would either be empty string or null depending on when/where the user was created so it won't be null anywhere after this change.
| verify(exactly = 0) { startMainActivity.invoke() } | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
Unit Tests for SDK change.
| // Init user logging | ||
| updateLoggingPrefs(account) | ||
| if (tokenMigration) { | ||
| userAccountManager.persistAccount(account) |
There was a problem hiding this comment.
This is the main SDK change. It avoids changing the current user and sending the user switch broadcast that should not be done for token migration.
| * separate process that Espresso and Compose Test APIs cannot access. | ||
| */ | ||
| class CustomTabPageObject { | ||
| class ChromeCustomTabPageObject(composeTestRule: ComposeTestRule): LoginPageObject(composeTestRule) { |
There was a problem hiding this comment.
The AdvancedAuthLoginTests are not included in this PR (still a little org setup to finish and a Firebase complication to work around) but will be in the next.
| val isFtl: Boolean by lazy { | ||
| Settings.System.getString( | ||
| InstrumentationRegistry.getInstrumentation().targetContext.contentResolver, | ||
| /* name = */ "firebase.test.lab" | ||
| ) == "true" | ||
| } |
There was a problem hiding this comment.
This nifty trick allows us to do things like set different timeout values for local vs CI runs or skip tests in certain scenarios (I am seeing a lot of instances of Chrome crashing on lower API levels in Firebase :/).
| import com.salesforce.samples.authflowtester.testUtility.KnownAppConfig.CA_OPAQUE | ||
| import org.junit.Rule | ||
|
|
||
| abstract class AuthFlowTest { |
There was a problem hiding this comment.
New base test class that handles all of the boiler-plate setup and common verification functions.
| val user: KnownUserConfig by lazy { | ||
| val minSdk = InstrumentationRegistry.getInstrumentation().targetContext | ||
| .applicationInfo.minSdkVersion | ||
| val userNumber = (Build.VERSION.SDK_INT - minSdk) % KnownUserConfig.values().count() | ||
| KnownUserConfig.values()[userNumber] | ||
| } |
There was a problem hiding this comment.
This might look a little strange, but it ensures each test runs uses a different user. By starting at minSdkVersion we can basically count up and never have a collision:
API 28 -> User 1
API 29 -> User 2
...
reusable-ui-workfow.yaml runs batches with a max size to match the number of users (for single user tests).
| val otherUser: KnownUserConfig by lazy { | ||
| val userNumber = (user.ordinal + 1) % KnownUserConfig.values().count() | ||
| KnownUserConfig.values()[userNumber] | ||
| } |
There was a problem hiding this comment.
Multi-User tests are run separately and the bash in reusable-ui-workfow.yaml groups API level to avoid collisions. So the groups look like:
[API 28, API 30], [API 32, API 34], ... [API 33, API 35], etc.
Since we can only run 2 levels at a time like this I should really add a 6th user, it would decrease test time for multi-user by 50%.
|
As always, the tests will not run with the latest yaml configuration for security (but I will update you with the results from my fork). @wmathurin @JohnsonEricAtSalesforce As I was experimenting and testing the new (complicated) bash batching that we could avoid the above limitation by moving all of the bash from yaml to scripts. As long as the name (and parameters) of those scripts don't change the updates would be immediately seen on PRs. As long as we don't run them before the permission check it should be safe. It could make local testing a bit easier too. Thoughts? |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #2847 +/- ##
============================================
+ Coverage 64.56% 64.60% +0.03%
Complexity 2925 2925
============================================
Files 222 222
Lines 17338 17352 +14
Branches 2474 2475 +1
============================================
+ Hits 11195 11210 +15
+ Misses 4936 4934 -2
- Partials 1207 1208 +1
🚀 New features to boost your workflow:
|
1 participant
Tests added:
BootConfigLoginTests- tests that use the (as currently defined) CA Opaque app in AuthFlowTester'sbootconfig.xmlas is.CAScopeSelectionLoginTests- Connected App tests with mixed scopes, flow and hybrid token options.ECALoginTests- ECA Opaque and JWT tests with various login options.BeaconLoginTests- Beacon Opaque and JWT tests with various login options.MultiUserLoginTests- Tests with multiple users logged in. Different app types, scopes, etc are used.The new
testMultiUser_tokenMigration_backgroundUsertest uncovered the fact that token migration would always set the specified user as the current user. That has been fixed in this PR and unit tests have been added to cover the SDK code change.To make the above test possible the test app UI was updated to allow for user selection when migrating:
Other Noteworthy Changes:
README.mdthat describes the UI Tests coverage and how to use the app for manual testing.ui_test_config.json. All users will be used at once for single user tests and theMultiUserLoginTestsare batched to run as many as possible but avoid runs using the same user simultaneously.There are separate WIs for Advanced Auth and Welcome test that will be finished soon.