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

[#4365] [#4907] Add test cases related to issue #4365 #5175

Merged
merged 1 commit into from
Mar 14, 2025

Conversation

kfcripps
Copy link
Contributor

#5123 made it very easy to add tests for functionality that is dependent on command-line options, so this PR does that and adds the following test cases:

This also closes #4907.

Signed-off-by: Kyle Cripps <[email protected]>
Copy link
Contributor

@ChrisDodd ChrisDodd left a comment

Choose a reason for hiding this comment

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

Looks good.

For future work, maybe make @command_line errors not trigger the full command line help message?

@kfcripps
Copy link
Contributor Author

Looks good.

For future work, maybe make @command_line errors not trigger the full command line help message?

@ChrisDodd I don't think that's related to @command_line. Even without using @command_line, the full command line help message gets printed for this error:

[--Werror=invalid] error: Error type-error cannot be demoted.

@ChrisDodd
Copy link
Contributor

Looks good.
For future work, maybe make @command_line errors not trigger the full command line help message?

@ChrisDodd I don't think that's related to @command_line. Even without using @command_line, the full command line help message gets printed for this error:

[--Werror=invalid] error: Error type-error cannot be demoted.

It's related to command line processing -- the help list occurs whenever there is an error from that. We probably want to be more sparing in general, as that help list is large and causes the actual error message to scroll off the screen. Mostly just an annoyance, however.

@fruffy fruffy added core Topics concerning the core segments of the compiler (frontend, midend, parser) infrastructure Topics related to code style and build and test infrastructure. and removed core Topics concerning the core segments of the compiler (frontend, midend, parser) labels Mar 13, 2025
Copy link
Collaborator

@fruffy fruffy left a comment

Choose a reason for hiding this comment

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

Lovely.

@fruffy
Copy link
Collaborator

fruffy commented Mar 14, 2025

Mostly just an annoyance, however.

I agree, I often run into this problem and the error message gets completely lost in the help output.

@kfcripps kfcripps added this pull request to the merge queue Mar 14, 2025
Merged via the queue into p4lang:main with commit 6b2cdaf Mar 14, 2025
23 checks passed
@kfcripps kfcripps deleted the 4365-4907-tests branch March 14, 2025 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Topics related to code style and build and test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add mechanism for easily adding tests for command-line options
3 participants