Skip to content

runtests: allow comments in setenv section, merge sections in test433 - #22389

Closed
vszakats wants to merge 1 commit into
curl:masterfrom
vszakats:one-setenv
Closed

runtests: allow comments in setenv section, merge sections in test433#22389
vszakats wants to merge 1 commit into
curl:masterfrom
vszakats:one-setenv

Conversation

@vszakats

@vszakats vszakats commented Jul 25, 2026

Copy link
Copy Markdown
Member

Comments were silently allowed before this patch, but internally they
were handled as env deletion commands, which just failed or did not
do harm. I figure it's better to allow explicitly and not pass to OS.

Copilot AI review requested due to automatic review settings July 25, 2026 12:19
@vszakats vszakats added the tests label Jul 25, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Updates the curl test runner to support comment lines inside <setenv> sections in test definitions, and simplifies test433 by consolidating its environment setup into a single <setenv> block.

Changes:

  • Teach tests/runner.pm to ignore #... comment lines when processing <setenv>.
  • Merge the duplicated <setenv> sections in tests/data/test433 and keep the explanatory note as an inline comment.
  • Keep the COLUMNS=300 adjustment within the main env block to avoid word-wrapped output during the test.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/runner.pm Skips # comment lines while parsing <setenv> entries so comments don’t get treated as environment variable names.
tests/data/test433 Consolidates env setup into one <setenv> section and adds an inline comment for the COLUMNS override.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@vszakats

vszakats commented Jul 25, 2026

Copy link
Copy Markdown
Member Author

It appears we have an upstream CI meltdown.

@vszakats vszakats changed the title runtests: allow comments setenv section, merge sections in test433 runtests: allow comments in setenv section, merge sections in test433 Jul 25, 2026
@vszakats vszakats closed this in b2a5369 Jul 26, 2026
@vszakats
vszakats deleted the one-setenv branch July 26, 2026 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

3 participants