Skip to content

Conversation

@theacodes
Copy link

@waprin Note that this is being merged into the pytest-refactor branch, which we'll merge into master when all is done.

Also, question for you: I can put all of the test functions in a class and mark the whole class as flaky. That'll keep me from having to mark each test case as flaky. Your call.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 24, 2016
@theacodes theacodes force-pushed the pytest-refactor-step-2 branch from 92efef1 to 562ab5f Compare February 24, 2016 23:15
# created and delete them during tearDown.
self.original_create = self.model.create
@py.test.yield_fixture(params=['datastore', 'cloudsql', 'mongodb'])
def app(request, monkeypatch):
Copy link

Choose a reason for hiding this comment

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

monkeypatch here looks vestigial

Copy link
Author

Choose a reason for hiding this comment

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

That's a... shockingly accurate way of putting it.

@jerjou
Copy link

jerjou commented Feb 25, 2016

LGTM
travis hates it though.

@theacodes
Copy link
Author

Travis didn't fail on this particular folder, so I'm ignoring it.

Will wait for @waprin to get a chance to review.

@theacodes theacodes force-pushed the pytest-refactor-step-2 branch from 562ab5f to fed573e Compare February 25, 2016 18:29
@theacodes
Copy link
Author

@jerjou @waprin I could structure the tests like this:

@py.test.yield_fixture(params=['datastore', 'cloudsql', 'mongodb'])
def app(request):
    ...


@py.test.yield_fixture
def model(monkeypatch, app):
    ...


@flaky(rerun_filter=flaky_filter)
@py.test.mark.usefixtures('app', 'model')
class TestCrudOperations(object):
    def test_list(self, app, model):
        ...

    def test_add(app):
        ...

    def test_edit(app, model):
        ...

    def test_delete(app, model):
        ...

This would require only one use of flaky, and would remove the unused model fixed from test_add while still making sure the fixture is used to clean up any created resources. WDYT?

@jerjou
Copy link

jerjou commented Feb 25, 2016

Either way - as long as there's a comment alluding to the dependency used for cleanup, so future-self won't go on a refactoring spree that messes things up mysteriously :)

@waprin
Copy link
Contributor

waprin commented Feb 25, 2016

These tests are awesome, I would just like to see the pytest magic explained.

@theacodes
Copy link
Author

These tests are awesome, I would just like to see the pytest magic explained.

I think the class-layout will help with that (usesfixtures helps), but I do want to avoid teaching py.test or talking down to the developer.

Let me switch to that, I think it's all-around easier to grok.

@theacodes theacodes force-pushed the pytest-refactor-step-2 branch from fed573e to 70b6c60 Compare February 25, 2016 22:23
@theacodes
Copy link
Author

@waprin @jerjou re-organized and added lots of comments and docstrings.

@theacodes theacodes force-pushed the pytest-refactor-step-2 branch from 70b6c60 to d1467ce Compare February 25, 2016 22:37
@waprin
Copy link
Contributor

waprin commented Feb 25, 2016

lgtm

@theacodes
Copy link
Author

Woohoo.

theacodes pushed a commit that referenced this pull request Feb 25, 2016
Refactoring 2-structured-data tests to use py.test
@theacodes theacodes merged commit 0bfc9ba into pytest-refactor Feb 25, 2016
@theacodes theacodes deleted the pytest-refactor-step-2 branch February 25, 2016 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants