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

Jump to bottom

gh-98393: Update test_os for bytes-like types #98487

Merged
merged 1 commit into from Nov 2, 2022
Merged

gh-98393: Update test_os for bytes-like types #98487

merged 1 commit into from Nov 2, 2022

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Oct 20, 2022 •
edited by bedevere-bot

@bedevere-bot bedevere-bot added awaiting core review tests Tests in the Lib/test dir labels Oct 20, 2022
@vstinner vstinner mentioned this pull request Oct 20, 2022
Merged
@vstinner
Copy link
Member Author

vstinner commented Oct 20, 2022

cc @serhiy-storchaka

vstinner
vstinner commented Oct 20, 2022
View changes
Lib/test/test_os.py Outdated
@@ -3861,17 +3861,17 @@ def test_oserror_filename(self):
for filenames, func, *func_args in funcs:
for name in filenames:
if not isinstance(name, (str, bytes)):
with self.assertRaises(TypeError):
func(name, *func_args)
# don't test TypeError here
Copy link
Member Author

@vstinner vstinner Oct 20, 2022

Choose a reason for hiding this comment

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

Other types are already well tested in other test methods of test_os.

Copy link
Member

@serhiy-storchaka serhiy-storchaka Oct 20, 2022

Choose a reason for hiding this comment

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

Then why these filenames are occurred here at first place? bytearray and memoryview should not be added in self.bytes_filenames.

BTW, does checking condition sys.platform == "win32" above still have sense?

Copy link
Member Author

@vstinner vstinner Nov 2, 2022

Choose a reason for hiding this comment

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

I removed bytearray and memoryviews from self.bytes_filenames (and self.filenames).

I removed the code specific to Windows: let's see how CIs like my updated change :-)

@bedevere-bot bedevere-bot mentioned this pull request Nov 2, 2022
Closed
@vstinner
Copy link
Member Author

vstinner commented Nov 2, 2022

@serhiy-storchaka: Would you mind to review the updated PR?

@vstinner
gh-98393: Update test_os for bytes-like types
fee1750 
Address Serhiy Storchaka's review.
@vstinner
Copy link
Member Author

vstinner commented Nov 2, 2022

"Tests / Check if generated files are up to date (pull_request)" failed for an unrelated reason:

python3.8 ./Tools/build/update_file.py ./Include/internal/pycore_ast.h ./Include/internal/pycore_ast.h.new
  File "./Tools/build/generate_opcode_h.py", line 111
    with (open(outfile, 'w') as fobj, open(internaloutfile, 'w') as iobj):
                             ^
SyntaxError: invalid syntax

I rebased my PR to see if it helps.

@vstinner vstinner merged commit f3007ac into python:main Nov 2, 2022
13 checks passed
@vstinner vstinner deleted the test_os branch Nov 2, 2022
@vstinner
Copy link
Member Author

vstinner commented Nov 2, 2022

Thanks for the review @serhiy-storchaka.

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

@serhiy-storchaka serhiy-storchaka

Assignees
No one assigned
Labels
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.

None yet
3 participants
@vstinner @serhiy-storchaka @bedevere-bot