Skip to content

Fix flaky telephony pool tests#7403

Merged
mitchellhenke merged 2 commits intomainfrom
mitchellhenke/fix-telephony-flakey-test
Nov 29, 2022
Merged

Fix flaky telephony pool tests#7403
mitchellhenke merged 2 commits intomainfrom
mitchellhenke/fix-telephony-flakey-test

Conversation

@mitchellhenke
Copy link
Contributor

🛠 Summary of changes

I think #7314 introduced some mutation of the constant that can affect other tests and may not always clean it up which can lead to failed specs elsewhere (example)

To reproduce the flaky test reliably, run rspec spec/lib/telephony/pinpoint/sms_sender_spec.rb spec/lib/telephony/telephony_spec.rb --seed 8116, which should result in:

Failures:

  1) Telephony.phone_info with pinpoint adapter uses the pinpoint adapter
     Failure/Error:
       response = client.phone_number_validate(
         number_validate_request: { phone_number: phone_number },
       )

     NoMethodError:
       undefined method `phone_number_validate' for #<Pinpoint::MockClient:0x00007f94fb854e30 @config=#<struct Telephony::PinpointSmsConfiguration application_id="fake-pinpoint-application-id-sms", shortcode="123456", country_code_longcode_pool={"PR"=>["+19393334444"]}, region="fake-pinpoint-region-sms", access_key_id="fake-pnpoint-access-key-id-sms", secret_access_key="fake-pinpoint-secret-access-key-sms", credential_role_arn=nil, credential_role_session_name=nil, credential_external_id=nil>>

This PR tries to ensure that all of the client pool mocking is reset in this spec file.

[skip changelog]
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, good catch, thanks!

Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
@mitchellhenke mitchellhenke merged commit 42588fa into main Nov 29, 2022
@mitchellhenke mitchellhenke deleted the mitchellhenke/fix-telephony-flakey-test branch November 29, 2022 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants