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

Fix the indeterminate test issue. #791

Merged
merged 1 commit into from Mar 31, 2017

Conversation

codebycliff
Copy link
Collaborator

Description

This pull request fixes the indeterminate test issue causing builds to fail randomly. I finally got to the bottom of what was causing it. The issue was with the minitest spec tests. A couple of the describe blocks didn't have the word "decorator" in them. This prevented minitest from registering the spec type as Draper::TestCase. Since the tests weren't registered as a Draper::TestCase, it wasn't including Draper::TestCase::ViewContextTeardown which defines teardown to clear the view context. Since the view context wasn't being cleared after these tests, the next test that called a helper on ViewContext.current blew up.

Testing

Once merged, we should not longer see builds fail randomly. You can test locally by:

  1. $ cd spec/dummy
  2. SEED=8910 rake should fail repeatedly on master.
  3. Switch to this branch and run the above command and the test suite should pass.

To-Dos

None

References

The issue was due to a couple describe blocks
not matching the definition in .
Since their name didn't match,
wasn't getting included in the minitest-specs. Since the
teardown didn't exist, the view context leaked to the
next tests to be ran.
@codebycliff codebycliff merged commit bcbfed6 into master Mar 31, 2017
@codebycliff codebycliff deleted the feature/fix-indeterminate-test branch March 31, 2017 19:25
@syguer
Copy link
Contributor

syguer commented Apr 1, 2017

WTF...
Great job 👍

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.

None yet

2 participants