Skip to content

Add print_heading testsuite helper. NFC#8650

Open
sbc100 wants to merge 1 commit intomainfrom
print_heading
Open

Add print_heading testsuite helper. NFC#8650
sbc100 wants to merge 1 commit intomainfrom
print_heading

Conversation

@sbc100
Copy link
Copy Markdown
Member

@sbc100 sbc100 commented Apr 27, 2026

No description provided.

@sbc100 sbc100 requested a review from a team as a code owner April 27, 2026 20:17
@sbc100 sbc100 requested review from stevenfontanella and removed request for a team April 27, 2026 20:17
Comment thread scripts/test/shared.py


def print_heading(msg, last=False):
global first
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we avoid the global and just print an extra newline here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think I ended up having to do this because:

  1. Some tests suites call print_heading mulitple times.
  2. We want the extra newlines also when running the suites as part of auto_update_tests.py

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It seems that we don't need it for point 1 because only the first call would need the preceding newline (we know within the function whether to insert a newline or not) and for point 2, we could similarly add the newline here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If we have a bunch of print_heading within a single suite I think the expectation is that there will be an empty line between those sub-sections of tests.. at least I imagine that is useful?

With your suggestion this PR would effectively change that behavior. Maybe that is fine?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants