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

Simplify the async-related tests #324

Merged
merged 7 commits into from Jul 4, 2022
Merged

Simplify the async-related tests #324

merged 7 commits into from Jul 4, 2022

Conversation

glunkad
Copy link
Contributor

@glunkad glunkad commented Jul 3, 2022

Implemented a fixture for the "skip async test" function since all test in that file will requires Flask > 2.0
suggested by jonasps and greyli
apiflask/tests/test_async.py
Before

def skip_flask1(app):
    if not hasattr(app, 'ensure_sync'):
        pytest.skip('This test requires Flask 2.0 or higher')

After

@pytest.fixture(autouse=True)
def skip_async_test(app):
    if not hasattr(app, 'ensure_sync'):
        pytest.skip('This test requires Flask 2.0 or higher')

To improve it

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
  • Add or update relevant docs, in the docs folder and in code docstring.
  • Add an entry in CHANGES.md summarizing the change and linking to the issue and your username.
  • Add *Version changed* or *Version added* note in any relevant docs and docstring.
  • Run pytest and tox, no tests failed.

@glunkad glunkad closed this Jul 3, 2022
@glunkad glunkad reopened this Jul 3, 2022
@greyli
Copy link
Member

greyli commented Jul 3, 2022

The fixture with autouse=True will be called automatically for each test function, you don't need to call it.

@jonasps
Copy link

jonasps commented Jul 3, 2022

I think this looks good now 👍🏻

@glunkad
Copy link
Contributor Author

glunkad commented Jul 4, 2022

@greyli
I am unable to debug the lint error.

@jonasps
Copy link

jonasps commented Jul 4, 2022

@9gl looks like it might be because you removed the blank line at the end of the file. Not sure.

@greyli
Copy link
Member

greyli commented Jul 4, 2022

@9gl It's Ok, I will take care of this later.

For development, you can refer to the contributing guide. The tox command will do the lint check (and the unit tests), and you can fix these issues based on the output. If you skip the local tox check, the GitHub CI will run them as well. You can click the "Details" link on the failing CI check to see the detailed log. For example, now the log shows:

tests/test_async.py:107:1: W293 blank line contains whitespace

It indicated that line 107 in tests/test_async.py contains some whitespace, so you will need to remove them. By the way, don't remove the blank line at the end of the files.

@greyli greyli changed the title Changed pytest autouse fixture in /tests/test_async.py Simplify the async-related tests Jul 4, 2022
@greyli greyli merged commit 874f7ad into apiflask:main Jul 4, 2022
@greyli
Copy link
Member

greyli commented Jul 4, 2022

Merged, thanks!

I squashed the commits, so you will need to reset your main branch:

git remote add upstream https://github.com/apiflask/apiflask
git fetch upstream
git reset --hard upstream/main

Next time please use different branches to submit PRs (check out the contributing guide).

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.

Use pytest autouse fixture in /tests/test_async.py
3 participants