fix(functional-tests): Update sms client to check for target #18514
Conversation
| @@ -38,7 +38,7 @@ export abstract class BaseTarget { | |||
| ); | |||
| this.authClient = this.createAuthClient(keyStretchVersion); | |||
| this.emailClient = new EmailClient(emailUrl); | |||
| this.smsClient = new SmsClient(); | |||
| this.smsClient = new SmsClient(this.name); | |||
There was a problem hiding this comment.
It's not accepted to access an abstract property in the constructor. We could make this a lazy getter though.
There was a problem hiding this comment.
Noticed exactly this after pushing... Updated with explicit initialization of the sms client before seeing your comment, but I can see now that a lazy getter might be preferred since we only use the sms client for a subset of tests.
There was a problem hiding this comment.
There were a couple issues with the way Twilio vs Redis were configured. I ended up just making the config really granular and added the ability to override per testing environment. This way we can probably get out of just about any weird situation.
See my PR over here. We can either land and cherry pick that entire PR, or do a partially cherry pick on the changes of that PR on the functional-tests folder.
Because: * We only want to set up the twilio client in CI if the target is stage or prod otherwise use redis This commit: * Check for target name Closes #
vpomerleau
force-pushed
the
attempt-funct-test-twilio-client-fix
branch
from
4288e1e to
930a9b0
Compare
|
Closing in favour of #18500 |
Because
This pull request
Issue that this pull request solves
Closes: (issue number)
Checklist
Put an
xin the boxes that applyScreenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
Any other information that is important to this pull request.