FTUE - Stage selection and Registration action testing #6091

ouchadam · 2022-05-18T13:06:36Z

Type of change

Feature
Bugfix
Technical
Other :

Content

Adds unit tests around the registration stages

Lifts the next stage logic out of the UI layer and into a dedicated class RegistrationActionHandler
Extracts all the RegistrationAction -> Stage logic out of the ViewModel and adds unit tests

Motivation and context

To add missing coverage to the FTUE area

Screenshots / GIFs

No UI changes

Tests

Sign up for a new account

Tested devices

Physical
Emulator
OS version(s): 28

ouchadam · 2022-05-18T13:07:56Z

vector/src/main/java/im/vector/app/features/onboarding/RegistrationActionHandler.kt

 import javax.inject.Inject

-class RegistrationActionHandler @Inject constructor() {
+class RegistrationActionHandler @Inject constructor(


the original implementation in RegistrationActionHandler has been extracted and renamed to RegistrationWizardActionDelegate

github-actions · 2022-05-18T14:03:47Z

Unit Test Results

150 files +  4 150 suites +4 2m 34s ⏱️ +2s
245 tests +  9 245 ✔️ +  9 0 💤 ±0 0 ❌ ±0
824 runs +36 824 ✔️ +36 0 💤 ±0 0 ❌ ±0

Results for commit 680b9a0. ± Comparison against base commit e6beb73.

This pull request removes 5 and adds 14 tests. Note that renamed tests count towards both.

im.vector.app.features.onboarding.OnboardingViewModelTest ‑ given personalisation enabled and registration has started and has dummy step to do, when handling action, then ignores other steps and does dummy
im.vector.app.features.onboarding.RegistrationActionHandlerTest ‑ given adding an email ThreePid fails with 401, when handling register action, then infer EmailSuccess
im.vector.app.features.onboarding.RegistrationActionHandlerTest ‑ given email verification errors with 401 and succeeds, when checking email validation, then continues to poll until success
im.vector.app.features.onboarding.RegistrationActionHandlerTest ‑ given email verification errors with 401 then fatal error, when checking email validation, then continues to poll until non 401 error
im.vector.app.features.onboarding.RegistrationActionHandlerTest ‑ when handling register action then delegates to wizard

im.vector.app.features.onboarding.RegistrationActionHandlerTest ‑ given flow result contains unsupported stages, when handling action, then returns UnsupportedStage
im.vector.app.features.onboarding.RegistrationActionHandlerTest ‑ given flow result with mandatory and optional stages, when handling action, then returns mandatory stage
im.vector.app.features.onboarding.RegistrationActionHandlerTest ‑ given flow result with missing stages, when handling action, then returns MissingNextStage
im.vector.app.features.onboarding.RegistrationActionHandlerTest ‑ given flow result with only optional dummy stage, when handling action, then returns MissingNextStage
im.vector.app.features.onboarding.RegistrationActionHandlerTest ‑ given flow result with only optional stages, when handling action, then returns optional stage
im.vector.app.features.onboarding.RegistrationActionHandlerTest ‑ given matrix org homeserver and flow result with missing mandatory stages, when handling action, then returns email item first
im.vector.app.features.onboarding.RegistrationActionHandlerTest ‑ given non matrix org homeserver and flow result with missing mandatory stages, when handling action, then returns first item
im.vector.app.features.onboarding.RegistrationActionHandlerTest ‑ given password already sent and missing mandatory dummy stage, when handling action, then fast tracks the dummy stage
im.vector.app.features.onboarding.RegistrationActionHandlerTest ‑ given wizard delegate returns success, when handling action, then returns success
im.vector.app.features.onboarding.RegistrationActionHandlerTest ‑ when processing SendAgainThreePid, then ignores result
…

♻️ This comment has been updated with latest results.

Claire1817 · 2022-06-08T08:58:01Z

vector/src/main/java/im/vector/app/features/onboarding/RegistrationWizardActionDelegate.kt

+    is Success      -> RegistrationResult.Complete(session)
+}
+
+sealed interface RegistrationResult {


I don't understand what is the role of this interface ?
Which difference with: RegistrationWizardAction.Result ?
And why did you put Result in the Delegate class ?

there's two layers/domains, the main difference is the split between generic SDK logic and Element specific things.

RegistrationWizardActionDelegate which handles the SDK flows (eg a 401 when sending the verification email is not actually an error), could argue this should be inside the SDK itself but wanted to avoid too much scope creep

RegistrationActionHandler which is Element's integration with the onboarding stages, technically it only needs to process the NextStep, the other result types are passed through. Element reorders the onboarding steps for Matrix.org, ignores certain results from the wizard and simplifies the flow so that clients of RegistrationActionHandler only know about the next Stage

The Results are needed because both layers have multiple different outcomes

Element's integration layer provides additional cases around the handling of the next registration step

StartRegistration NextStage MissingNextStage UnsupportedStage RegistrationComplete SendEmailSuccess Error Ignored

SDK

Error Complete NextStep SendEmailSuccess

Hopefully the tests help show the difference 🤞 SDK Delegate | Element intergration

thanks for the explanation :)

…ion and adding tests - renames the existing handler to a wizard delegate

sonarqubecloud · 2022-06-09T17:27:18Z

SonarCloud Quality Gate failed.

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

fedrunov

LGTM

fedrunov · 2022-06-14T09:48:36Z

vector/src/main/java/im/vector/app/features/onboarding/RegistrationActionHandler.kt

-    data class AddThreePid(val threePid: RegisterThreePid) : RegisterAction
-    object SendAgainThreePid : RegisterAction
+    private fun findNextStage(state: SelectedHomeserverState, flowResult: FlowResult): Result {
+        val orderedResult = when {


I think if would be much more readable here. when doesn't look good with complex conditions. But I won't insist

I've been using when specifically for those types of cases 🙈

ouchadam added the Z-Tests label May 18, 2022

ouchadam commented May 18, 2022

View reviewed changes

ouchadam changed the base branch from feature/adm/matrix-org-ordering to base/adm/email-veriifcation-and-ordering May 19, 2022 11:12

ouchadam force-pushed the feature/adm/ftue-register-test-cases branch from fb6a5b2 to 0a13e9a Compare May 19, 2022 11:12

ouchadam mentioned this pull request May 20, 2022

FTUE - Msisdn (phone number) entry #6108

Merged

6 tasks

ouchadam changed the base branch from base/adm/email-veriifcation-and-ordering to develop May 23, 2022 20:25

ouchadam force-pushed the feature/adm/ftue-register-test-cases branch from 0a13e9a to 171056e Compare May 24, 2022 15:29

ouchadam marked this pull request as ready for review May 24, 2022 16:27

ouchadam requested review from a team, Florian14 and ariskotsomitopoulos and removed request for a team May 24, 2022 16:27

ouchadam force-pushed the feature/adm/ftue-register-test-cases branch from 171056e to 680b9a0 Compare June 1, 2022 09:37

ouchadam requested review from a team, Claire1817, Florian14 and fedrunov and removed request for ariskotsomitopoulos, a team and Florian14 June 1, 2022 12:49

Claire1817 reviewed Jun 8, 2022

View reviewed changes

ouchadam force-pushed the feature/adm/ftue-register-test-cases branch from 0fd67ba to 6d281ff Compare June 8, 2022 09:37

ouchadam added 2 commits June 9, 2022 08:52

extracting registration action business logic to the handler abstract…

ba18c6f

…ion and adding tests - renames the existing handler to a wizard delegate

replacing comment extracted function (also convered by a test case)

ef1356f

ouchadam added 2 commits June 9, 2022 08:52

fixing import ordering

88167a0

renaming success type to something more concrete

befcfe8

ouchadam force-pushed the feature/adm/ftue-register-test-cases branch from 6d281ff to befcfe8 Compare June 9, 2022 07:52

formatting

6681d4f

fedrunov approved these changes Jun 14, 2022

View reviewed changes

ouchadam requested a review from Claire1817 June 14, 2022 15:25

Claire1817 approved these changes Jun 16, 2022

View reviewed changes

ouchadam merged commit b78fb53 into develop Jun 16, 2022

ouchadam deleted the feature/adm/ftue-register-test-cases branch June 16, 2022 09:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FTUE - Stage selection and Registration action testing #6091

FTUE - Stage selection and Registration action testing #6091

ouchadam commented May 18, 2022 •

edited

Loading

ouchadam May 18, 2022

github-actions bot commented May 18, 2022 •

edited

Loading

Claire1817 Jun 8, 2022

ouchadam Jun 8, 2022 •

edited

Loading

Claire1817 Jun 16, 2022

sonarqubecloud bot commented Jun 9, 2022

fedrunov left a comment

fedrunov Jun 14, 2022 •

edited

Loading

ouchadam Jun 14, 2022

FTUE - Stage selection and Registration action testing #6091

FTUE - Stage selection and Registration action testing #6091

Conversation

ouchadam commented May 18, 2022 • edited Loading

Type of change

Content

Motivation and context

Screenshots / GIFs

Tests

Tested devices

ouchadam May 18, 2022

Choose a reason for hiding this comment

github-actions bot commented May 18, 2022 • edited Loading

Unit Test Results

Claire1817 Jun 8, 2022

Choose a reason for hiding this comment

ouchadam Jun 8, 2022 • edited Loading

Choose a reason for hiding this comment

Claire1817 Jun 16, 2022

Choose a reason for hiding this comment

sonarqubecloud bot commented Jun 9, 2022

fedrunov left a comment

Choose a reason for hiding this comment

fedrunov Jun 14, 2022 • edited Loading

Choose a reason for hiding this comment

ouchadam Jun 14, 2022

Choose a reason for hiding this comment

ouchadam commented May 18, 2022 •

edited

Loading

github-actions bot commented May 18, 2022 •

edited

Loading

ouchadam Jun 8, 2022 •

edited

Loading

fedrunov Jun 14, 2022 •

edited

Loading