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

gh-115421: Test that our Makefile has all needed test folders #115813

Merged
merged 10 commits into from
Mar 7, 2024

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Feb 22, 2024

This PR is the alternative to #115511

While we would still have to keep adding directories to TESTSUBDIRS manually, we would now have a test to catch any cases where people forget to update / remove these entries.

This test works with --disable-test-modules as well.

Which approach do you like best?

@sobolevn
Copy link
Member Author

I've pushed a demo commit: f048fb5

It is missing test/archivetestdata from Makefile, some tests should fail. Let's see!

@sobolevn
Copy link
Member Author

@sobolevn sobolevn added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 22, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit 182822d 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 22, 2024
@sobolevn
Copy link
Member Author

Related failure:

======================================================================
FAIL: test_makefile_test_folders (test.test_tools.test_makefile.TestMakefile.test_makefile_test_folders)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/buildbot/buildarea/pull_request.ware-freebsd/build/Lib/test/test_tools/test_makefile.py", line 79, in test_makefile_test_folders
    self.check_existing_test_modules(actual_result)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^
  File "/buildbot/buildarea/pull_request.ware-freebsd/build/Lib/test/test_tools/test_makefile.py", line 61, in check_existing_test_modules
    self.assertEqual(
    ~~~~~~~~~~~~~~~~^
        unique_test_dirs.symmetric_difference(set(used)),
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        set(),
        ^^^^^^
    )
    ^
AssertionError: Items in the first set but not the second:
''

@sobolevn sobolevn added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 25, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @sobolevn for commit 001c14b 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Feb 25, 2024
@sobolevn
Copy link
Member Author

@Yhg1s it is ready :)

Copy link
Member

@encukou encukou left a comment

Choose a reason for hiding this comment

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

LGTM, with two suggestions:

Lib/test/test_tools/test_makefile.py Outdated Show resolved Hide resolved
Lib/test/test_tools/test_makefile.py Outdated Show resolved Hide resolved
Co-authored-by: Petr Viktorin <encukou@gmail.com>
@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 6, 2024
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @encukou for commit 6caa8f9 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Mar 6, 2024
@encukou encukou merged commit 72dbea2 into python:main Mar 7, 2024
111 of 113 checks passed
@sobolevn
Copy link
Member Author

sobolevn commented Mar 7, 2024

Thanks everyone!

@bedevere-app
Copy link

bedevere-app bot commented Mar 7, 2024

GH-116462 is a backport of this pull request to the 3.12 branch.

1 similar comment
@bedevere-app
Copy link

bedevere-app bot commented Mar 7, 2024

GH-116462 is a backport of this pull request to the 3.12 branch.

Yhg1s pushed a commit that referenced this pull request Mar 7, 2024
…H-115813) (#116462)

* gh-115421: Test that our Makefile has all needed test folders (GH-115813)

* Update the list of installed test subdirectories

---------

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
encukou added a commit to encukou/cpython that referenced this pull request Mar 8, 2024
…hem (pythonGH-115813) (pythonGH-116462)

* pythongh-115421: Test that our Makefile has all needed test folders (pythonGH-115813)
* Update the list of installed test subdirectories

---------

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
@bedevere-app
Copy link

bedevere-app bot commented Mar 8, 2024

GH-116498 is a backport of this pull request to the 3.11 branch.

encukou added a commit that referenced this pull request Mar 11, 2024
…H-115813)

This backports:
- GH-115813
- GH-115422

Unlike on the main branch, new directories are added to the end,
so they're a bit easier to patch out if a redistributor needs to do so.

On main & 3.12, there's a special case for `idlelib/idle_test`; on
3.11 TESTSUBDIRS has several more entries that are not in `test/`.
This backport ignores all of them (including idlelib).
(The alternative would be list them, as additions to TEST_HOME_DIR.
But that's probably too invasive; people might split stdlib up in
surprising ways.)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
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.

None yet

4 participants