GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-131178: Add tests for `sysconfig` command-line interface by LamentXU123 · Pull Request #139934 · python/cpython

Conversation

@LamentXU123

@LamentXU123 LamentXU123 commented Oct 11, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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())
  • Issue: Add tests for command-line interface #131178

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
LamentXU123 added 3 commits October 11, 2025 12:51
@LamentXU123
Update test_sysconfig.py
5fcc793
@LamentXU123
Update test_sysconfig.py
663642c
@LamentXU123
Update test_sysconfig.py
9a01167
@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
Add tests for command-line interface #131178
Open
7 of 9 tasks
LamentXU123 added 3 commits October 11, 2025 13:14
@LamentXU123
Fix lint CI
85d0bc4
@LamentXU123
fix CI
ba7362d
@LamentXU123
fix lint
fd39af0
@StanFromIreland StanFromIreland added the skip news label Oct 11, 2025
cmaloney
cmaloney reviewed Oct 11, 2025
View reviewed changes

@cmaloney cmaloney left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this 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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_sysconfig.py
check=True
)
self.assertTrue(output.returncode == 0)
self.assertTrue(output.stdout.startswith("Platform: "))

@cmaloney cmaloney Oct 11, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@LamentXU123 LamentXU123 Oct 12, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_sysconfig.py

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

@cmaloney cmaloney Oct 11, 2025

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
LamentXU123 reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@LamentXU123 LamentXU123 Oct 12, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@LamentXU123

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.
cmaloney reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@LamentXU123 LamentXU123 closed this Oct 12, 2025
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

Uh oh!

There was an error while loading. Please reload this page.

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.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@LamentXU123 @cmaloney @StanFromIreland
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.