Clarify `lit`'s definition of failure and conditions when it exits with exit code 1 #136190

i-ky · 2025-04-17T20:12:58Z

Documentation of when lit exits with code 1 is out of date.

It is no longer just "FAIL or XPASS", there are more failure types:

Lines 51 to 55 in b30100b

    
           # Failures 
        
           UNRESOLVED = ResultCode("UNRESOLVED", "Unresolved", True) 
        
           TIMEOUT = ResultCode("TIMEOUT", "Timed Out", True) 
        
           FAIL = ResultCode("FAIL", "Failed", True) 
        
           XPASS = ResultCode("XPASS", "Unexpectedly Passed", True)

Exit code can also be affected by --ignore-fail option:

llvm-project/llvm/utils/lit/lit/main.py

Lines 154 to 162 in b30100b

    
           has_failure = any(t.isFailure() for t in discovered_tests) 
        
           if has_failure: 
        
               if opts.ignoreFail: 
        
                   sys.stderr.write( 
        
                       "\nExiting with status 0 instead of 1 because " 
        
                       "'--ignore-fail' was specified.\n" 
        
                   ) 
        
               else: 
        
                   sys.exit(1)

This PR extracts a clear definition of "failure" from the description of --report-failures-only option:

llvm-project/llvm/docs/CommandGuide/lit.rst

Lines 194 to 196 in b30100b

    
           .. option:: --report-failures-only 
        
            Only include unresolved, timed out, failed and unexpectedly passed tests in the report.

...puts it into "Test Status Results" section and references it when describing exit codes and --ignore-fail option.

P.S. Note that above description was adjusted based on discussion below.

github-actions · 2025-04-17T20:13:16Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2025-04-17T20:13:51Z

@llvm/pr-subscribers-testing-tools

Author: None (i-ky)

Changes

Documentation of when lit exits with code is out of date.

It is no longer just "FAIL or XPASS", there are more failure types:

llvm-project/llvm/utils/lit/lit/Test.py

Lines 51 to 55 in b30100b

    
           # Failures 
        
           UNRESOLVED = ResultCode("UNRESOLVED", "Unresolved", True) 
        
           TIMEOUT = ResultCode("TIMEOUT", "Timed Out", True) 
        
           FAIL = ResultCode("FAIL", "Failed", True) 
        
           XPASS = ResultCode("XPASS", "Unexpectedly Passed", True)

Exit code can also be affected by --ignore-fail option:

llvm-project/llvm/utils/lit/lit/main.py

Lines 154 to 162 in b30100b

    
           has_failure = any(t.isFailure() for t in discovered_tests) 
        
           if has_failure: 
        
               if opts.ignoreFail: 
        
                   sys.stderr.write( 
        
                       "\nExiting with status 0 instead of 1 because " 
        
                       "'--ignore-fail' was specified.\n" 
        
                   ) 
        
               else: 
        
                   sys.exit(1)

Full diff: https://github.com/llvm/llvm-project/pull/136190.diff

1 Files Affected:

(modified) llvm/docs/CommandGuide/lit.rst (+4-2)

diff --git a/llvm/docs/CommandGuide/lit.rst b/llvm/docs/CommandGuide/lit.rst
index 812e1d819e9c8..6cb1ee4627254 100644
--- a/llvm/docs/CommandGuide/lit.rst
+++ b/llvm/docs/CommandGuide/lit.rst
@@ -384,8 +384,10 @@ ADDITIONAL OPTIONS
 EXIT STATUS
 -----------
 
-:program:`lit` will exit with an exit code of 1 if there are any FAIL or XPASS
-results.  Otherwise, it will exit with the status 0.  Other exit codes are used
+:program:`lit` will exit with an exit code of 1 if there are any failures (i.e.
+unresolved, timed out, failed or unexpectedly passed tests, see also
+:option:`--report-failures-only`) and :option:`--ignore-fail` has not been
+passed.  Otherwise, it will exit with the status 0.  Other exit codes are used
 for non-test related failures (for example a user error or an internal program
 error).

jh7370

I'm not sure --report-failures-only should be considered a canonical reference of what is considered a "failure" any more than the list you've provided here is. More generally, I don't think it's useful to attempt to specify a comprehensive list of failure kinds, as it could easily rot, if somebody extended the set of failures with a new variety.

How about "... an exit code of 1, if there are any failure results, for example FAIL or XPASS results. Otherwise, it will ..."?

i-ky · 2025-04-22T17:42:14Z

Hi @jh7370! Thank you for your review! ❤️

I'm not sure --report-failures-only should be considered a canonical reference of what is considered a "failure" any more than the list you've provided here is.

I have mentioned --report-failures-only only because it had a correct (i.e. matching the source code) definition of "failure". I agree that neither --report-failures-only nor "Exit Status" section should be the authoritative source for the definition of "failure". The term is used across the entire llvm/docs/CommandGuide/lit.rst and it would make sense to define the term on the neutral territory. "Test Status Results" section may be a good place.

More generally, I don't think it's useful to attempt to specify a comprehensive list of failure kinds, as it could easily rot, if somebody extended the set of failures with a new variety.

I understand the concern of the docs rotting over time, but I would still insist on providing an exact definition given the importance of the term "failure" in the context of a testing framework. lit is available as a standalone tool and is being used outside of LLVM project. Providing a complete documentation is very important for end users and integrations.

(For example, I am working on a project that uses lit for tests. I needed the exact definition of "failure" to set up a Problem Matched for VS Code. I may try to build a VS Code extension later. Since there were conflicting definitions of "failure" in "Exit Status" section and in the description of --report-failures-only I had to look into the source code.)

Please let me know what you think about moving the definition of "failure" to "Test Status Results" section and referencing it from both "Exit Status" and the description of --report-failures-only? I think it reduces the risk of the rot to the minimum, since "Test Status Result" will inevitably get updated when adding new ResultCode and from there it will be easy to spot that "failure" definition needs an update too.

If you don't find my above points reasonable enough I am OK to adjust as you suggested:

How about "... an exit code of 1, if there are any failure results, for example FAIL or XPASS results. Otherwise, it will ..."?

jh7370 · 2025-04-23T06:23:21Z

Please let me know what you think about moving the definition of "failure" to "Test Status Results" section and referencing it from both "Exit Status" and the description of --report-failures-only?

This sounds like a good plan to me. We'll probably need to tweak some wording of the individual statuses.

i-ky · 2025-04-24T20:52:52Z

@jh7370, could you please have another look?

jh7370

You might want to tweak the PR description given this revised version of the patch. Otherwise, LGTM. Do you need me to merge this for you?

i-ky · 2025-04-25T18:57:15Z

Hi @jh7370!

You might want to tweak the PR description given this revised version of the patch.

Sure, done.

Otherwise, LGTM.

Yay! 🥳 Thank you for your suggestions! ❤️

Do you need me to merge this for you?

Yes, please. I am not a maintainer, I don't have merge permissions.

github-actions · 2025-04-28T06:07:00Z

@i-ky Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

…th exit code 1 (llvm#136190) Documentation of when `lit` exits with code 1 is out of date. It is no longer just "FAIL or XPASS", there are more failure types: https://github.com/llvm/llvm-project/blob/b30100b87f24847afd6407b4939a184ebcf16ef9/llvm/utils/lit/lit/Test.py#L51-L55 Exit code can also be affected by `--ignore-fail` option: https://github.com/llvm/llvm-project/blob/b30100b87f24847afd6407b4939a184ebcf16ef9/llvm/utils/lit/lit/main.py#L154-L162 This PR extracts a clear definition of "failure" from the description of `--report-failures-only` option: https://github.com/llvm/llvm-project/blob/b30100b87f24847afd6407b4939a184ebcf16ef9/llvm/docs/CommandGuide/lit.rst?plain=1#L194-L196 ...puts it into "Test Status Results" section and references it when describing exit codes and `--ignore-fail` option.

Clarify conditions of lit exiting with exit code 1

2a6e543

llvmbot added the testing-tools label Apr 17, 2025

jh7370 reviewed Apr 22, 2025

View reviewed changes

Define "failure" in "Test Status Results"

802c663

jh7370 approved these changes Apr 25, 2025

View reviewed changes

i-ky changed the title ~~Clarify conditions of lit exiting with exit code 1~~ Clarify lit's definition of failure and conditions when it exits with exit code 1 Apr 25, 2025

jh7370 merged commit 6f6af49 into llvm:main Apr 28, 2025
10 checks passed

i-ky deleted the fix-lit-exit-code-docs branch April 28, 2025 11:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Clarify `lit`'s definition of failure and conditions when it exits with exit code 1 #136190

Clarify `lit`'s definition of failure and conditions when it exits with exit code 1 #136190

Uh oh!

i-ky commented Apr 17, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Apr 17, 2025

Uh oh!

llvmbot commented Apr 17, 2025

Uh oh!

jh7370 left a comment

Uh oh!

i-ky commented Apr 22, 2025

Uh oh!

jh7370 commented Apr 23, 2025

Uh oh!

i-ky commented Apr 24, 2025

Uh oh!

jh7370 left a comment

Uh oh!

i-ky commented Apr 25, 2025

Uh oh!

Uh oh!

github-actions bot commented Apr 28, 2025

Uh oh!

Uh oh!

	# Failures
	UNRESOLVED = ResultCode("UNRESOLVED", "Unresolved", True)
	TIMEOUT = ResultCode("TIMEOUT", "Timed Out", True)
	FAIL = ResultCode("FAIL", "Failed", True)
	XPASS = ResultCode("XPASS", "Unexpectedly Passed", True)

	has_failure = any(t.isFailure() for t in discovered_tests)
	if has_failure:
	if opts.ignoreFail:
	sys.stderr.write(
	"\nExiting with status 0 instead of 1 because "
	"'--ignore-fail' was specified.\n"
	)
	else:
	sys.exit(1)

	.. option:: --report-failures-only

	Only include unresolved, timed out, failed and unexpectedly passed tests in the report.

Clarify lit's definition of failure and conditions when it exits with exit code 1 #136190

Clarify lit's definition of failure and conditions when it exits with exit code 1 #136190

Uh oh!

Conversation

i-ky commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Apr 17, 2025

Uh oh!

llvmbot commented Apr 17, 2025

Uh oh!

jh7370 left a comment

Choose a reason for hiding this comment

Uh oh!

i-ky commented Apr 22, 2025

Uh oh!

jh7370 commented Apr 23, 2025

Uh oh!

i-ky commented Apr 24, 2025

Uh oh!

jh7370 left a comment

Choose a reason for hiding this comment

Uh oh!

i-ky commented Apr 25, 2025

Uh oh!

Uh oh!

github-actions bot commented Apr 28, 2025

Uh oh!

Uh oh!

Clarify `lit`'s definition of failure and conditions when it exits with exit code 1 #136190

Clarify `lit`'s definition of failure and conditions when it exits with exit code 1 #136190

i-ky commented Apr 17, 2025 •

edited

Loading