FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-131178: Add tests for `sysconfig` command-line interface by LamentXU123 · Pull Request #139934 · python/cpython · GitHub

/ cpython Public

gh-131178: Add tests for sysconfig command-line interface - #139934

Closed
LamentXU123 wants to merge 6 commits into
python:mainfrom
LamentXU123:add-test-for-sysconfig
ConversationCommits (6)ChecksFiles changed
Closed

gh-131178: Add tests for sysconfig command-line interface#139934
LamentXU123 wants to merge 6 commits into
python:mainfrom
LamentXU123:add-test-for-sysconfig

Conversation

LamentXU123 commented Oct 11, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

As doc points out, we can use sysconfig lib directly as a script to obtain configs.

This PR add tests for this feature.

PS: I notice that the CLI actually has a --generare-posix-vars attribute that is undocumented. I don't think we need to add test for that (maybe this is an internal function or what. If not I think we can add document for this later)

def _main():
    """Display all information sysconfig detains."""
    if '--generate-posix-vars' in sys.argv:
        _generate_posix_vars()
        return
    print(f'Platform: "{get_platform()}"')
    print(f'Python version: "{get_python_version()}"')
    print(f'Current installation scheme: "{get_default_scheme()}"')
    print()
    _print_dict('Paths', get_paths())
    print()
    _print_dict('Variables', get_config_vars())

LamentXU123 requested a review from FFY00 as a code owner October 11, 2025 05:06
bedevere-app Bot added tests Tests in the Lib/test dir awaiting review labels Oct 11, 2025
bedevere-app Bot mentioned this pull request Oct 11, 2025
Open
9 tasks
cmaloney reviewed Oct 11, 2025
View reviewed changes

cmaloney left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

👍 to not documenting --generate-posix-vars. Other comments inline

Comment thread Lib/test/test_sysconfig.py
check=True
)
self.assertTrue(output.returncode == 0)
self.assertTrue(output.stdout.startswith("Platform: "))

cmaloney Oct 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The docs say that all these values should be present in the text, could we check for them in the test? That way the test validates they're there (and if the output changes, know it needs to change)

get_platform(), get_python_version(), get_path() and get_config_vars().

LamentXU123 Oct 12, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I will add a stricter test case by mocking an output and directly compare the mock output and the real one.

Comment thread Lib/test/test_sysconfig.py

class CommandLineTests(unittest.TestCase):
def test_config_output(self):
output = subprocess.run(

cmaloney Oct 11, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

From what I can see in the base issue the pattern for these tests is to use contextlib.redirect_stdout + call the function which implements main directly (sysconfig._main) rather than run / use a subprocess.

See for example: https://github.com/python/cpython/pull/131275/files#diff-eabc91c9e7a2586ffc6ca849d6636e74e28825a4515745c280b045dbdf857e39R723-R727

That should also fix the WASI check which is currently failing with: OSError: [Errno 58] wasi does not support processes.

LamentXU123 Oct 12, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ah! Thank you. I am really stuck on how to fix the WASI test.

LamentXU123 commented Oct 12, 2025

Copy link
Copy Markdown
Contributor Author

Actually we've already got this for the CLI test:

    def test_main(self):
        # just making sure _main() runs and returns things in the stdout
        with captured_stdout() as output:
            _main()
        self.assertTrue(len(output.getvalue().split('\n')) > 0)

So we don't need to change things(why are the CLI test mixing with others XD) .Sorry that I overlook this.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

cmaloney cmaloney left review comments
FFY00 Awaiting requested review from FFY00 FFY00 is a code owner

Assignees

No one assigned

Labels

awaiting review skip news tests Tests in the Lib/test dir

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL