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

Pytest/parametrize test simple #1146

Closed
wants to merge 30 commits into from

Conversation

palao
Copy link
Contributor

@palao palao commented Oct 22, 2023

The old test_simple.py has been parametrized and rewritten as test_baseline.py + conftest.py. Now, instead of one large test, it is 28 smaller tests that have been pytest-marked with ft.

@thesamesam
Copy link
Member

Try rebasing on master?

robbat2 and others added 24 commits October 23, 2023 10:42
factor out aux constants and function. This commit is a refactor that
will help to transform SimpleEmergeTestCase into a parametrized pytest
test.

Signed-off-by: David Palao <[email protected]>
port to pytest. BREAKING CHANGE: This is the first, rudimentary
version of ``test_simple.py`` that works exclusively with pytest.

Signed-off-by: David Palao <[email protected]>
Add initial conftest.py

It contains a basic ``playground`` fixture, which is parametrized
to yield a ``ResolverPlayground`` instance for each value of
``SUPPORTED_GENTOO_BINPKG_FORMATS``.

Signed-off-by: David Palao <[email protected]>
Add async_loop and binhost fixtures.
This is a refactor: some setup code is moved from conftest.py to
test_simple.py.

Signed-off-by: David Palao <[email protected]>
Add simple_command parametrized fixture. This is work in progress:
in order to include post-checks to some tests, a different approach
is needed.

Signed-off-by: David Palao <[email protected]>
Complete pytest parametrization. The test_simple_emerge is completely
parametrized in commands and in binary formats. This commit is the first
implementation. Further refactorings/improvements shall follow.

Signed-off-by: David Palao <[email protected]>
Remove spurious definition of commands. The ``simple_command`` fixture
was repeating the definition of some commands. That's not needed.

Signed-off-by: David Palao <[email protected]>
Fix issue in simple_command fixture. It was bad-defined in case
the argument parser missed the ability to parse intermixed args.
Also that command was renamed to make it more descriptive.

Signed-off-by: David Palao <[email protected]>
Add some markers: ft, unit and stress. And mark
test_simple.test_simple_emerge with ft, such that those tests can be
triggered with::

   pytest -m ft

Signed-off-by: David Palao <[email protected]>
Refactor:

- _TEST_COMMAND_*NAMES -> _SIMPLE_COMMAND_*SEQUENCE
- Added NOOP
- defined global constant

Signed-off-by: David Palao <[email protected]>
Add docstrings. They contain some brief documentation about the tests,
the fixture with the commands and how to select those tests with pytest.

Also the main test function was renamed:

test_simple_emerge -> test_portage_baseline

Signed-off-by: David Palao <[email protected]>
Add a new fixture. It adds one more layer of indirection to the
``simple_command`` fixture. Quick benchmarks tell that it is
faster due to pytest caching.

Signed-off-by: David Palao <[email protected]>
Refactor.
Portage commands converted into fixtures. Now the commands are
simply tuples of strings. I pretend them to become instances of
a Command class that can encapsulate the context of each command
more cleanly.

Signed-off-by: David Palao <[email protected]>
Refactor.
Removed parameter from function since it's constant.
Signed-off-by: David Palao <[email protected]>
Refactor.
Introduced a PortageCommand class to be able to pack together a
sequence of operations that, as a whole, express expected behavior
in portage.
(This commit is a baseline: the refactor works out, but further
refactors must follow).

Signed-off-by: David Palao <[email protected]>
Refactor.
Removed some dead code.

Signed-off-by: David Palao <[email protected]>
Add PortageCommandSequence. Some commands must run in sequence
to test some functionality of portage. This class has been added
to chain commands within a test.

Signed-off-by: David Palao <[email protected]>
Split the test commands in monoliths. Each monolith should be an
indivisible sequence of commands that test some complex
functionality of portage.

Run the tests with::

   pytest -m ft

if pytest-xdist is installed, it can be used to speed up::

   pytest -m ft -n 8

Signed-off-by: David Palao <[email protected]>
Refactor. Removed comments and changed simple -> baseline.

Signed-off-by: David Palao <[email protected]>
@palao palao force-pushed the pytest/parametrize-testSimple branch from 446fdc9 to b794dc3 Compare October 23, 2023 10:32
@palao
Copy link
Contributor Author

palao commented Oct 23, 2023

It fails miserably, but I don't understand the error. It seems to be meson related?

@@ -36,16 +36,10 @@ jobs:
python -VV
python -m site
python -m pip install --upgrade pip
# setuptools needed for 3.12+ because of https://github.com/mesonbuild/meson/issues/7702.
Copy link
Member

Choose a reason for hiding this comment

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

You seem to have undone the hack we were doing here before. Any particular reason or was it accidental?

I think that explains the CI failure.

Copy link
Member

Choose a reason for hiding this comment

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

I think you want to drop 9284e0d from your branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the delay. It was accidental, of course. No idea why it ended up in my branch...

I'll remove it and force push.

NEWS Outdated
@@ -156,6 +156,8 @@ to meson-python, thanks to Chewi!

Breaking changes:
* The minimum supported Python version is now >= Python 3.9.
* Portage now installed with Meson and Python sdist + wheel now prepared with
Copy link
Member

Choose a reason for hiding this comment

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

Drop -- merge conflict error.

Copy link
Member

Choose a reason for hiding this comment

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

@@ -0,0 +1,79 @@
#!/usr/bin/env python
Copy link
Member

Choose a reason for hiding this comment

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

Drop again? Looks like a merge conflict error (we don't want runTests.py now, right?)

Copy link
Member

Choose a reason for hiding this comment

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

This makes Portage PEP 517 compliant.

When building via meson-python, the man pages and logrotate config are
no longer included as there seems little point.

Bug: https://bugs.gentoo.org/910035
Signed-off-by: James Le Cuirot <[email protected]>
Signed-off-by: Sam James <[email protected]>
zmedico and others added 4 commits October 26, 2023 16:10
When a UTF-8 locale, or UTF-8 mode is detected, set
portage.utf8_mode to True, and do not wrap file access
with _unicode_func_wrapper. This is intended to mitgate
issues with byte string handling in python libraries
like shutil, as reported in bug 914722.

This patch is intended to be a simple and minimal
implementation that can be optimized later through
the elimination of unecessary encoding/decoding.

The str() wrapping in the unit tests is for lazily
evaluated instances of lazy_value, which is used to
account for mock portage.const.EPREFIX values that
exist during unit tests.

Bug: https://bugs.gentoo.org/914722
Signed-off-by: Zac Medico <[email protected]>
port to pytest. BREAKING CHANGE: This is the first, rudimentary
version of ``test_simple.py`` that works exclusively with pytest.

Signed-off-by: David Palao <[email protected]>
Add some markers: ft, unit and stress. And mark
test_simple.test_simple_emerge with ft, such that those tests can be
triggered with::

   pytest -m ft

Signed-off-by: David Palao <[email protected]>
It has been renamed to test_baseline.py

Signed-off-by: David Palao <[email protected]>
@palao palao force-pushed the pytest/parametrize-testSimple branch from b794dc3 to 0316cea Compare October 26, 2023 14:13
...since the file had been renamed.

Signed-off-by: David Palao <[email protected]>
@palao
Copy link
Contributor Author

palao commented Oct 26, 2023

Did I do what you wanted me to do?
At least now it works.
Sorry for the mess!
What is next?

@thesamesam thesamesam requested a review from zmedico October 27, 2023 01:04
Copy link
Member

@zmedico zmedico left a comment

Choose a reason for hiding this comment

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

Looks great! Thanks!

I tried pytest-xdist with pytest -m ft -n 8 and it was fast!

@thesamesam
Copy link
Member

thesamesam commented Oct 30, 2023

Thank you Zac and thank you David for this fantastic work!

I often find the tests kind of painful to sit and wait for, so this is a great boon.

@thesamesam
Copy link
Member

pytest -n 32 --dist=worksteal took 65s for me!

@palao
Copy link
Contributor Author

palao commented Oct 30, 2023

I'm very glad! Thank you.

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.

5 participants