Skip to content

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

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
Closed
LamentXU123 wants to merge 6 commits into from
Closed

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

LamentXU123 wants to merge 6 commits into from

Conversation

@LamentXU123
Copy link
Contributor

@LamentXU123 LamentXU123 commented Oct 11, 2025 •
edited
Loading

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

image

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 LamentXU123 requested a review from FFY00 as a code owner October 11, 2025 05:06
@bedevere-app bedevere-app bot added tests Tests in the Lib/test dir awaiting review labels Oct 11, 2025
@bedevere-app bedevere-app bot mentioned this pull request Oct 11, 2025
Open
9 tasks
cmaloney
cmaloney reviewed Oct 11, 2025
View reviewed changes
Copy link
Contributor

@cmaloney cmaloney left a comment

Choose a reason for hiding this comment

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

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

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

@cmaloney cmaloney Oct 11, 2025

Choose a reason for hiding this comment

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

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().

Copy link
Contributor Author

@LamentXU123 LamentXU123 Oct 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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

Lib/test/test_sysconfig.py

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

@cmaloney cmaloney Oct 11, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@LamentXU123 LamentXU123 Oct 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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

@LamentXU123
Copy link
Contributor Author

LamentXU123 commented Oct 12, 2025

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cmaloney cmaloney cmaloney left review comments

@FFY00 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

@LamentXU123 @cmaloney @StanFromIreland