[go: nahoru, domu]
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.

Sign up for GitHub

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

Jump to bottom

Make tests pytest and unittest compatible #254

Merged
merged 1 commit into from
Dec 16, 2022
Merged

Make tests pytest and unittest compatible #254

merged 1 commit into from
Dec 16, 2022

Conversation

codesue
Copy link
Collaborator
@codesue codesue commented Dec 3, 2022

What does this pull request do?

This pull request enables running tests with pytest and unittest as the test runners. Previously, running tests with these runners raised the following error:

absl.flags._exceptions.UnparsedFlagAccessError: Trying to access flag --test_tmpdir before flags were parsed.

This happened because the absl flags were only being parsed when running the modules directly. With this change, flags are manually parsed and use default values if the modules aren't run directly, as is the case when using pyttest and unittest. See the absl-py flags documentation for a more detailed explanation.

How did you test this change?

pytest

Ran pytest model_card_toolkit. Abridged summary:

================================== test session starts ===================================
platform darwin -- Python 3.8.13, pytest-7.2.0, pluggy-1.0.0
rootdir: /Users/sue/workspace/oss/mct-dev/model-card-toolkit
plugins: anyio-3.6.2
collected 78 items                                                                       

model_card_toolkit/core_test.py ..................                                 [ 23%]
model_card_toolkit/model_card_test.py ................                             [ 43%]
model_card_toolkit/documentation/examples/cats_vs_dogs_test.py .                   [ 44%]
model_card_toolkit/utils/graphics_test.py .........                                [ 56%]
model_card_toolkit/utils/json_util_test.py ...                                     [ 60%]
model_card_toolkit/utils/tfx_util_test.py .....................                    [ 87%]
model_card_toolkit/utils/validation_test.py .......                                [ 96%]
model_card_toolkit/utils/testdata/testdata_utils_test.py ...                       [100%]

==================================== warnings summary ====================================
../../../../.pyenv/versions/3.8.13/envs/mct-dev/lib/python3.8/site-packages/hdfs/config.py:15
  /Users/sue/.pyenv/versions/3.8.13/envs/mct-dev/lib/python3.8/site-packages/hdfs/config.py:15: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
    from imp import load_source

../../../../.pyenv/versions/3.8.13/envs/mct-dev/lib/python3.8/site-packages/jupyter_client/connect.py:27
  /Users/sue/.pyenv/versions/3.8.13/envs/mct-dev/lib/python3.8/site-packages/jupyter_client/connect.py:27: DeprecationWarning: Jupyter is migrating its paths to use standard platformdirs
  given by the platformdirs library.  To remove this warning and
  see the appropriate new directories, set the environment variable
  `JUPYTER_PLATFORM_DIRS=1` and then run `jupyter --paths`.
  The use of platformdirs will be the default in `jupyter_core` v6
    from jupyter_core.paths import jupyter_data_dir

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
======================= 78 passed, 2 warnings in 83.81s (0:01:23) ========================

unittest

Couldn't figure out the right pattern to discover all tests, so I only tested the problematic modules.

Ran python -m unittest model_card_toolkit/core_test.py:

Ran 18 tests in 72.494s

OK

Ran python -m unittest model_card_toolkit/utils/testdata/tfxtest.py:

Ran 1 test in 0.002s

OK

How did you document this change?

N/A

Before submitting

Before submitting a pull request, please be sure to do the following:

  • Read the How to Contribute guide if this is your first contribution.
  • Open an issue or discussion topic to discuss this change. (Discussed over video chat.)
  • Write new tests if applicable.
  • Update documentation if applicable.

@codesue codesue mentioned this pull request Dec 3, 2022
Closed
4 tasks
@codesue codesue mentioned this pull request Dec 3, 2022
Merged
4 tasks
@codesue codesue mentioned this pull request Dec 10, 2022
Closed
4 tasks
@codesue
Copy link
Collaborator Author
codesue commented Dec 16, 2022

Hi @casassg, @deutranium, or @hanneshapke, would you please review this when you get a chance. 🙏

deutranium
deutranium approved these changes Dec 16, 2022
View reviewed changes
Copy link
Collaborator
@deutranium deutranium left a comment

Choose a reason for hiding this comment

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

Lgtm, thanks for contributing!!

@codesue codesue merged commit 313b268 into tensorflow:master Dec 16, 2022
@codesue codesue deleted the codesue/make-pytest-compatible branch December 16, 2022 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deutranium deutranium deutranium approved these changes

@hanneshapke hanneshapke Awaiting requested review from hanneshapke

@casassg casassg Awaiting requested review from casassg

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Improve the contributor experience
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@codesue @deutranium