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-103379: Fix up old tests for `pathlib.PurePath._parse_path` by barneygale · Pull Request #103380 · python/cpython

Conversation

@barneygale

@barneygale barneygale commented Apr 8, 2023 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

These tests now make assertions about the public PurePath.drive, root and parts attributes, rather than an internal method. They're moved out of the _BaseFlavourTest, PosixFlavourTest and NTFlavourTest classes, which are removed.

  • Issue: Vestigial pathlib 'flavour' tests make assertions about internal method #103379

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@barneygale
pythonGH-103379: Fix up old tests for pathlib.PurePath._parse_parts()
2fc2192 
These tests now make assertions about the public `PurePath.drive`, `root`
and `parts` attributes, rather than an internal method. They're moved out
of the `_BaseFlavourTest`, `PosixFlavourTest` and `NTFlavourTest` classes,
which are removed.
@barneygale barneygale added tests Tests in the Lib/test dir skip news topic-pathlib labels Apr 8, 2023
@bedevere-bot bedevere-bot mentioned this pull request Apr 8, 2023
Vestigial pathlib 'flavour' tests make assertions about internal method #103379
Closed
@bedevere-bot bedevere-bot added the awaiting core review label Apr 8, 2023
@barneygale barneygale mentioned this pull request Apr 8, 2023
GH-101362: Omit path anchor from pathlib.PurePath()._parts #102476
Merged
@barneygale
barneygale requested a review from AlexWaygood April 8, 2023 18:05
terryjreedy
terryjreedy reviewed Apr 8, 2023
View reviewed changes

@terryjreedy terryjreedy left a comment

Copy link
Copy Markdown
Member

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 have seen discussions of whether or not to write tests for private functions and am somewhat on the fence. Since you propose to merely change the existing tests, rather than delete them, and the change makes it clearer (to me) what is really being tested, I approve.

Sorry, something went wrong.

Uh oh!

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

Uh oh!

There was an error while loading. Please reload this page.
terryjreedy
terryjreedy reviewed Apr 8, 2023
View reviewed changes
Comment thread Lib/test/test_pathlib.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@terryjreedy terryjreedy changed the title GH-103379: Fix up old tests for pathlib.PurePath._parse_parts() GH-103379: Fix up old tests for pathlib.PurePath._parse_path Apr 8, 2023
@barneygale
Convert lists to tuples
9542694
terryjreedy
terryjreedy reviewed Apr 8, 2023
View reviewed changes

@terryjreedy terryjreedy left a comment

Copy link
Copy Markdown
Member

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 calls with first and last args being flat tuples looks fine. This looks better to me than before.

Sorry, something went wrong.

Uh oh!

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

Uh oh!

There was an error while loading. Please reload this page.
barneygale and others added 2 commits April 9, 2023 03:22
@barneygale @terryjreedy
Update Lib/test/test_pathlib.py
572d152 
Co-authored-by: Terry Jan Reedy <[email protected]>
@barneygale
Fix tests; use assignment operator.
61eca57
AlexWaygood
AlexWaygood approved these changes Apr 9, 2023
View reviewed changes

@AlexWaygood AlexWaygood left a comment

Copy link
Copy Markdown
Member

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.

Looks great to me -- thanks @barneygale!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Apr 9, 2023
@barneygale
barneygale merged commit 0a675f4 into python:main Apr 9, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Apr 9, 2023
@barneygale

barneygale commented Apr 9, 2023

Copy link
Copy Markdown
Contributor Author

Thank you both!
AlexWaygood reacted with heart emoji
All reactions
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
warsaw pushed a commit to warsaw/cpython that referenced this pull request Apr 11, 2023
@barneygale @terryjreedy @warsaw
pythonGH-103379: Fix up old tests for pathlib.PurePath._parse_path (p…
d05e4bc 
…ythonGH-103380)

Co-authored-by: Terry Jan Reedy <[email protected]>
aisk pushed a commit to aisk/cpython that referenced this pull request Apr 18, 2023
@barneygale @terryjreedy @aisk
pythonGH-103379: Fix up old tests for pathlib.PurePath._parse_path (p…
275f466 
…ythonGH-103380)

Co-authored-by: Terry Jan Reedy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@terryjreedy terryjreedy terryjreedy left review comments
@AlexWaygood AlexWaygood AlexWaygood approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

skip news tests Tests in the Lib/test dir topic-pathlib

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.

4 participants

@barneygale @terryjreedy @AlexWaygood @bedevere-bot
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.