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

test framework: bulk reformat #4669

Merged
merged 2 commits into from
Dec 29, 2024
Merged

Conversation

mwichmann
Copy link
Collaborator

@mwichmann mwichmann commented Dec 18, 2024

Includes test snippets contained in docstrings, too.

Note, once this is ready to merge, an additional commit should add the main commit hash to .git-blame-ignore-revs (don't know which hash to add until we know this won't change anymore).

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt and RELEASE.txt (and read the README.rst).
  • I have updated the appropriate documentation

@mwichmann mwichmann added the maintenance Tasks to maintain internal SCons code/tools label Dec 18, 2024
test = TestCmd.TestCmd(diff_stderr='diff_re')
print("diff_stderr:")
test.diff_stderr('a\nb.\nc\n', 'a\nbb\nc\n')
test.diff_stderr('a\\nb.\\nc\\n', 'a\\nbb\\nc\\n')
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this right? it's escaping the newline so it'll show up as \n instead of being a newline?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

missed that one... I didn't think reformatting would change that. Will check into.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, think the change is actually correct, as it's a string-in-a-string problem. Most of the other code snippets run via self.popen_python already did this escape-escaping.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's more than a handful of these changes in this file if I remember correctly. Were they broken before and just never tested?

if (doCheckLog and
logfile.find(
"scons: warning: The stored build information has an unexpected class.") >= 0):
if (
Copy link
Contributor

Choose a reason for hiding this comment

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

Odd formatting change..?

Imcludes test snippets contained in docstrings, too.

Signed-off-by: Mats Wichmann <[email protected]>
Insted of really long strings of repeated characters, use f-strings to
compose the "expected" output for the banner function tests.  This keeps
the code formatter from breaking things an ugly way, and is really more
"accurate" anyway.

Signed-off-by: Mats Wichmann <[email protected]>
@bdbaddog bdbaddog merged commit 540cb61 into SCons:master Dec 29, 2024
8 of 10 checks passed
@mwichmann mwichmann added this to the 4.9 milestone Dec 30, 2024
@mwichmann mwichmann deleted the framework-black branch December 30, 2024 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Tasks to maintain internal SCons code/tools
Projects
Status: Complete
Development

Successfully merging this pull request may close these issues.

2 participants