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

Non-deterministic tests #95

Merged

Conversation

marcelomorgado
Copy link
Contributor

Issue link

Refs #94

Types of changes

Bug fix (non-breaking change that fixes an issue)

Steps

1. Run tasit-action test suite 50 times. Results:

Testcase: Contract messages/transaction subscriptions tests should emit error event when orphan/uncle block occurs
Failed 9 times. Same error.

2. Skipping reliable slower for next rounds (saving 4s).

3. Run non-deterministic the test case 50 times.

Passed all times.
Note: Seems isolation doesn't work well.

4. Implement ganache-cli snapshot/revert in beforeEach / afterEach

5. Run tasit-action test suite 50 times again. Results:

All passed 😃 .

6. Came back with the skipped slow test case and ran test suite 10 times.

All passed.

7. Run tasit-sdk test suite 10 times.

All passed.

Notes:
- Oddly one of both test cases identified as non-deterministic ("should watch contract's ValueChanged event") doesn't failed at all.
- If non-deterministic tests still happen on CI, the next step will be using same container env locally to check.

@marcelomorgado
Copy link
Contributor Author

I've just see that just failed on my Circle CI 😠
Failed testcase was: "should watch contract's ValueChanged event"

I'll keep trying to find it out.

@pcowgill pcowgill changed the title Feature/non deterministic tests Non-deterministic tests Jan 9, 2019
@marcelomorgado
Copy link
Contributor Author

marcelomorgado commented Jan 10, 2019

  1. I've run testsuite 100 times, and all tests passed.
  2. I've following this https://circleci.com/docs/2.0/local-cli/ to simulate circle ci building on my local env, I've run testsuite 5 times, and all passed.
  3. I've rerun Circle CI workflow 5 times and failed 3 times.

Unfortunately, I've lost a lot of time trying to simulate non-deterministic failing locally.
Now I'll fixes using Circle CI platform.

@marcelomorgado
Copy link
Contributor Author

@pcowgill after trying a lot, I couldn't make this test to work well.
I've added that to quarantine to unlock CI and go ahead with higher priority issues.
I'll try to solve this soon.
Is it sounds good?

Note: Without this non-deterministic test I've rerun CI workflow 10 times and all passed.

@pcowgill
Copy link
Member

Re: your original steps when you created the issue

These findings are perplexing and interesting. Thanks for being so thorough here.

One take-home point is that we should continue to keep an eye out for continuing to make sure each test is isolated as one of our ongoing efforts going forward.

Often if a test isn't deterministic, there's a race condition for whether something async is finishing or not, and we're not properly awaiting the async thing before moving on. So let's keep an eye out for something like that. It might even turn out that it's related to #76 , but I would have to give that more thought to decide if it could make sense. That's just a quick gut reaction, so those might not even be possibly related.

Re: your second batch of comments

That's too bad it ended up being different when you set up the CircleCI env locally - sorry that wasn't more straightforward.

I've added that to quarantine to unlock CI and go ahead with higher priority issues.
I'll try to solve this soon.

That sounds like a sensible next step - thanks!

Note: Without this non-deterministic test I've rerun CI workflow 10 times and all passed.

Great!

@pcowgill
Copy link
Member

@marcelomorgado Oh, and despite us being stuck with some further things to investigate here, I meant to say that I'm really impressed with all the steps you took above!

@pcowgill
Copy link
Member

@marcelomorgado I’m merging this now so as not to interfere with any testing you’ll be doing on other micro-PRs you may want to open. But please check out the comments on this PR after the merge to see whether we should track any more issues. Thanks!

@pcowgill pcowgill merged commit 4bf875c into tasitlabs:develop Jan 10, 2019
@marcelomorgado marcelomorgado deleted the feature/non-deterministic-tests branch January 23, 2019 22:13
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