Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(fix): Fix sporadic whatsapp service test fails #429

Closed
wants to merge 2 commits into from

Conversation

nikoksr
Copy link
Owner

@nikoksr nikoksr commented Oct 20, 2022

Description

Increase the connectivity timeouts for the WhatsApp service and change the order of execution for some assertments in the testfile.

Motivation and Context

We're currently experiencing a bug in our CI where the whatsapp.New method fails inconsistently. The bug is not reproducible on a local machine, so I'm guessing here that too short timeouts may be the issue.

How Has This Been Tested?

Difficult to test since bug only occurs in GitHub's CI. I was so far not successful in reproducing it locally.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (no code change)
  • Refactor (refactoring production code)
  • Other

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

We're currently experiencing a bug in our CI where the whatsapp.New
method fails inconsistently. The bug is not reproducible on a local machine,
so I'm guessing here that too short timeouts may be the issue.
@nikoksr nikoksr added affects/services Issue or PR related to a notification service. affects/tests Issue or PR related to testing. labels Oct 20, 2022
@nikoksr
Copy link
Owner Author

nikoksr commented Oct 20, 2022

@svaloumas the Whatsapp service's test fails sporadically in CI, which is very annoying, since we have to keep re-running the failed CI job until it succeeds. It's most likely a false positive and I can't reproduce it locally, so I'm taking a "blind" guess here.

The timeouts may fix the bug itself but also the assert order may be beneficial, since asserting a nil-error first, would reveal to us the actual underlying error in case of a failed test.

Edit: Aaaand it failed immediatly. But, it did report the underlying error, which is: couldn't dial whatsapp web websocket: websocket: bad handshake. Looks like the better solution to just tackle #274 instead of keeping an already dead service "alive".

@nikoksr nikoksr closed this Oct 20, 2022
@nikoksr nikoksr deleted the test/fix-sporadic-whatsapp-fails branch October 25, 2022 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects/services Issue or PR related to a notification service. affects/tests Issue or PR related to testing. size/XS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant