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 an --offline option to pytest #1555

Merged
merged 1 commit into from
May 18, 2019
Merged

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented Apr 11, 2019

Usage:

$ py.test --offline

This option skip all tests that can not be run offline, as they are marked as "online". Either from manual tag:

@pytest.mark.online
def test_call_google():
    request.get('https://google.com')

Or from a command example:

@example('.title http://google.com', '[ Google ] - google.com', online=True)
def title_command(bot, trigger):
    # ...

This is especially useful for people like me who are always traveling and can't have access to a reliable Internet connection.

Existing online examples have been marked as such.

@Exirel Exirel added this to the 7.0.0 milestone Apr 11, 2019
@Exirel Exirel requested a review from dgw April 11, 2019 21:25
@dgw dgw added the Tests label Apr 11, 2019
@dgw
Copy link
Member

dgw commented Apr 11, 2019

Before I dive into this—I like the idea—we might get some utility from a set of options:

  • --offline, alias --offline-only: run only offline-safe tests
  • --online, alias --online-only: run only online tests (which call APIs or whatever)
  • --no-offline: skip offline tests
  • --no-online: skip online tests

This might be overkill, true. If it makes implementation easier we can do just the -only args and skip the no- options. My thinking here is we could later configure Travis to run only the offline tests during required-pass jobs, and have separate test runs for the online tests that are allowed to fail. Then we wouldn't have random transient changes to DuckDuckGo search results (or whatever) failing related PRs.

Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

LGTM. Needs only two things:

  • rebasing to avoid merge conflicts introduced on master since this PR was opened
  • adding the online arg to the docstring of the module.example class

As discussed on IRC, my ideas for a more complicated setup we could use to separate online tests (that might fail) in CI will come in a future PR, possibly after getting bikeshedded first in a dedicated issue.


I can't resist correcting the commit message, which contains an incorrect call to request.get. (Well, I guess it could be correct if the file does import requests as request, but why would you do this…)

And since I'm already commenting on that, I can also point out that "This option skip all tests" is missing something—and you will instantly know what. 😀 (There's also "can not" -> "cannot"—using that in two-word form is very uncommon in present-day writing, but it's not wrong per se.)

@Exirel
Copy link
Contributor Author

Exirel commented May 17, 2019

I got reminded of this PR when I had trouble testing locally because some random website was inaccessible and/or very slow.

@dgw I think this PR is becoming more and more important, since Travis can literally fails for the wrong reasons because of online-only tests.

@Exirel Exirel requested a review from dgw May 17, 2019 09:15
@dgw
Copy link
Member

dgw commented May 18, 2019

Hmm… @Exirel I think you did something weird in your last rebase. The commit is showing me as author, which can't be right. This code is all you. 😕

@Exirel
Copy link
Contributor Author

Exirel commented May 18, 2019

@dgw I've honestly no idea how or why. :|

Usage:

    $ py.test --offline

This option skips all tests that cannot be run offline, as they are
marked as "online". Either from manual tag:

    import requests

    @pytest.mark.online
    def test_call_google():
        requests.get('https://google.com')

Or from a command example:

    @example('.title http://google.com', '[ Google ] - google.com', online=True)
    def title_command(bot, trigger):
        # ...

This is especially useful for people like me who are always traveling
and can't have access to a reliable Internet connection. This could be
used by Travis to speed-up the test suite, and/or to make it more
reliable.

Existing online examples have been marked as such.
@Exirel
Copy link
Contributor Author

Exirel commented May 18, 2019

Yay git reset HEAD~1 worked as intended! Should be good now.

Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

We'll talk about further enhancements to reduce false CI failures for a future PR. 😸

@dgw dgw merged commit d417c54 into sopel-irc:master May 18, 2019
@Exirel Exirel deleted the tests-offline branch September 5, 2019 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants