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] Add synchronizer in UserStateFactory to avoid randomly failed #120

Merged
merged 2 commits into from
Oct 5, 2023

Conversation

Lisooo790926
Copy link
Collaborator

Please follow the guidelines in Contribution.md first, then start to create your pull request here.

Summary

Fix the randomly test failed issue

Linked Issue

#119

Details

add await before create method

Impacted Areas

signup test

Tests

Possible Impacts

signup test

Visual Materials

Verification Steps

Todo

Checklist

  • All new and existing tests pass
  • My changes do not introduce new security vulnerabilities
  • Run yarn lint:fix

@github-actions github-actions bot added test test improvements relay and removed test test improvements labels Sep 30, 2023
Copy link
Collaborator

@chentihe chentihe left a comment

Choose a reason for hiding this comment

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

I guess the problem is from the UserStateFactory. In the UserStateFactory, there's no synchronizer, while we initiate the UserStateFactory, the type of synchronizer is Synchronizer not UnirepSocialSynchronizer whose eventHandler does not contain UnirepApp events. Hence, even we execute userState.waitForSync(), the synchronizer does not sync the UserSignUp event.

@chentihe chentihe self-requested a review October 1, 2023 09:36
@Lisooo790926
Copy link
Collaborator Author

Lisooo790926 commented Oct 2, 2023

Thanks @chentihe, I think this is the root cause! Add one more commit for this UserStateFactory.
However, I think frontend will meet the same issue, I will leave await until synchorinzer has been changed in all createUserState.
reference:

Copy link
Contributor

@vivianjeng vivianjeng left a comment

Choose a reason for hiding this comment

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

you can init a user state without a synchronizer

packages/relay/test/utils/UserStateFactory.ts Outdated Show resolved Hide resolved
packages/relay/src/synchornizer.ts Outdated Show resolved Hide resolved
@netlify
Copy link

netlify bot commented Oct 4, 2023

Deploy Preview for unirep-social-taiwan canceled.

Name Link
🔨 Latest commit 3b604a1
🔍 Latest deploy log https://app.netlify.com/sites/unirep-social-taiwan/deploys/651d91b2a545ce0007bc1d7d

Copy link
Collaborator

@chentihe chentihe left a comment

Choose a reason for hiding this comment

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

The unit test will pass randomly because there's a UnirepSocialSynchronizer while the server starts. UnirepSocialSynchronizer will also fetch the new block to handle the event asynchronously. Because of this, the unit test may pass randomly. we still need to set UnirepSocialSynchronizer into UserState to prevent this error.

@Lisooo790926 Lisooo790926 changed the title [Test] Add await before create in handleUserSignup method [Test] Add synchronizer in UserStateFactory to avoid randomly failed Oct 5, 2023
@github-actions github-actions bot added the test test improvements label Oct 5, 2023
@vivianjeng vivianjeng merged commit 066b01a into main Oct 5, 2023
11 checks passed
@vivianjeng vivianjeng deleted the test_fix_random_testing_error branch October 5, 2023 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test test improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants