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

Fixing Testify to report test case results when it finishes #322

Merged
merged 3 commits into from Feb 3, 2016

Conversation

chunkyg
Copy link
Contributor

@chunkyg chunkyg commented Feb 2, 2016

Fixing a bug, testify stopped reporting/writing test case results(--test-case-results) in its latest version(v0.8.0). This change will fix the issue.

@asottile
Copy link
Contributor

asottile commented Feb 2, 2016

Please write a regression test so this doesn't break in the future -- I'd suggest an end-to-end test in test/plugins/test_case_time_log_test.py

You're also failing flake8 which means you didn't run the testsuite :(

# and not a function!). self.run is as good a method as any.
test_case_result = TestResult(self.run)
test_case_result.start()
self.fire_event(self.EVENT_ON_RUN_TEST_CASE, test_case_result)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this fake .run method necessary? This used to be just so the sql reporter could know when to stop so I removed it when I removed the sql reporter.

Copy link

Choose a reason for hiding this comment

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

let's get the tests in place and then refactor

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed flake8 failures and also added new tests which will do basic check to avoid breaking it in future.

We can write an integration tests in another change because it will require time and this current fix needs to be pushed asap. Created the ticket for this #323.

We need the fake .run for now because removing it is breaking TestResults. I will create a ticket for this once current changes are pushed.

@asottile
Copy link
Contributor

asottile commented Feb 2, 2016

An integration would actually be easier than what you wrote:

with tempdir() as tmpdir:
    tempfile = os.path.join(tempdir, 'tmp.json')
    subprocess.check_call((
        sys.executable, '-m', 'testify.test_program', '--test-case-results', tempfile, 'test/test_suite_subdir/define_testcase.py'
    ))
    with open(tempfile, 'r') as f:
        contents = json.load(f)
        T.assert_in('start_time', contents)
        T.assert_in('end_time', contents)

or something like that

@ydgholz
Copy link

ydgholz commented Feb 2, 2016

Let's release this and add the integration test to the followup issue (where we factor out the method param to TestResult)

ymilki added a commit that referenced this pull request Feb 3, 2016
Fixing Testify to report test case results when it finishes
@ymilki ymilki merged commit 61838d3 into Yelp:master Feb 3, 2016
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

4 participants