Skip to content

Failures are ignored if subsequent checks pass in single run #212

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.

Sign up for GitHub

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

Jump to bottom
Closed
B-McDonnell opened this issue Feb 2, 2025 · 2 comments · Fixed by #213
Closed

Failures are ignored if subsequent checks pass in single run #212

B-McDonnell opened this issue Feb 2, 2025 · 2 comments · Fixed by #213
Labels
bug Something isn't working

Comments

@B-McDonnell
Copy link
Contributor

describe your issue

When commit-check is run with multiple checks, failures are overridden by passing checks that run after the failures.

Example running only --message, which fails as expected:

hooks/commit-msg:

#!/bin/sh

MESSAGE_FILE="$1"

# check message only
commit-check --message "$MESSAGE_FILE"

example commit:

git commit -m "unconventional commit with a bad message!"
Commit rejected by Commit-Check.

  (c).-.(c)    (c).-.(c)    (c).-.(c)    (c).-.(c)    (c).-.(c)
   / ._. \      / ._. \      / ._. \      / ._. \      / ._. \
 __\( C )/__  __\( H )/__  __\( E )/__  __\( C )/__  __\( K )/__
(_.-/'-'\-._)(_.-/'-'\-._)(_.-/'-'\-._)(_.-/'-'\-._)(_.-/'-'\-._)
   || E ||      || R ||      || R ||      || O ||      || R ||
 _.' '-' '._  _.' '-' '._  _.' '-' '._  _.' '-' '._  _.' '-' '._
(.-./`-´\.-.)(.-./`-´\.-.)(.-./`-´\.-.)(.-./`-´\.-.)(.-./`-´\.-.)
 `-´     `-´  `-´     `-´  `-´     `-´  `-´     `-´  `-´     `-´

Commit rejected.

Type message check failed => unconventional commit with a bad message!

It doesn't match regex: ^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test){1}(\([\w\-\.]+\))?(!)?: ([\w ])+([\s\S]*)|(Merge).*|(fixup!.*)
The commit message should be structured as follows:

<type>[optional scope]: <description>
[optional body]
[optional footer(s)]

More details please refer to https://www.conventionalcommits.org
Suggest: please check your commit message whether matches above regex

git log:

72b8ebc (HEAD -> main) feat: Initial commit

Example running both --message and --author-name (with name set in git config) which succeeds unexpectedly:

hooks/commit-msg:

#!/bin/sh

MESSAGE_FILE="$1"

# check message and author name
commit-check --message --author-name "$MESSAGE_FILE"

Example commit:

$ git commit -m "unconventional commit with a bad message!"
Commit rejected by Commit-Check.

  (c).-.(c)    (c).-.(c)    (c).-.(c)    (c).-.(c)    (c).-.(c)
   / ._. \      / ._. \      / ._. \      / ._. \      / ._. \
 __\( C )/__  __\( H )/__  __\( E )/__  __\( C )/__  __\( K )/__
(_.-/'-'\-._)(_.-/'-'\-._)(_.-/'-'\-._)(_.-/'-'\-._)(_.-/'-'\-._)
   || E ||      || R ||      || R ||      || O ||      || R ||
 _.' '-' '._  _.' '-' '._  _.' '-' '._  _.' '-' '._  _.' '-' '._
(.-./`-´\.-.)(.-./`-´\.-.)(.-./`-´\.-.)(.-./`-´\.-.)(.-./`-´\.-.)
 `-´     `-´  `-´     `-´  `-´     `-´  `-´     `-´  `-´     `-´

Commit rejected.

Type message check failed => unconventional commit with a bad message!

It doesn't match regex: ^(build|chore|ci|docs|feat|fix|perf|refactor|revert|style|test){1}(\([\w\-\.]+\))?(!)?: ([\w ])+([\s\S]*)|(Merge).*|(fixup!.*)
The commit message should be structured as follows:

<type>[optional scope]: <description>
[optional body]
[optional footer(s)]

More details please refer to https://www.conventionalcommits.org
Suggest: please check your commit message whether matches above regex

[main 3601b27] unconventional commit with a bad message!
 1 file changed, 4 insertions(+)
 create mode 100755 hooks/pre-commit

git log:

3601b27 (HEAD -> main) unconventional commit with a bad message!
72b8ebc feat: Initial commit

Cause

The source of the bug is here: https://github.com/commit-check/commit-check/blame/91ee12b3f5045523cbdacd07390c0ce98f3b8231/commit_check/main.py#L109-L124

Annotated with previous example:

def main() -> int:
    """The main entrypoint of commit-check program."""
    parser = get_parser()
    args = parser.parse_args()
    retval = PASS

    with error_handler():
        config = validate_config(args.config) if validate_config(
            args.config,
        ) else DEFAULT_CONFIG
        checks = config['checks']
        if args.message:
            retval = commit.check_commit_msg(checks, args.commit_msg_file)  # retval is now FAIL
        if args.author_name:
            retval = author.check_author(checks, "author_name")             # retval is now PASS! Uh oh!
        if args.author_email:
            retval = author.check_author(checks, "author_email")
        if args.branch:
            retval = branch.check_branch(checks)
        if args.commit_signoff:
            retval = commit.check_commit_signoff(checks)
        if args.merge_base:
            retval = branch.check_merge_base(checks)

    if args.dry_run:
        retval = PASS
    return retval  # returns PASS

commit-check version

commit-check 0.9.2

.commit-check.yml


      		
    

      
  





        



            
            

              
            

        

      


      
    








      

    



      

  
            

    

      
    

    


      

          @B-McDonnell
B-McDonnell




          added
  the 

  bug

  Something isn't working
 label


      Feb 2, 2025

    

  








      

  
      



  

  @B-McDonnell





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      

  
    Contributor


      

  Author


  


  

    

      

      
            B-McDonnell
  

      

      

      commented


        Feb 2, 2025


      
  •

  

    
      

        
          edited
          
        
        
      

    
    
      
        
      Loading

    
  



    


  





      


        


  
    

      
          
Note that my example used commit-msg, so the author name is actually grabbed from the previous commit. A better example might be --check-signoff


Either way, author-name's success (against the previous commit) still overrode a failed --message check.

      		
    

  





        


            

            
            

              
            

        

      


      

            
  

    
  
    
    

  

  


    












      

  
          

  

    
  


  

      

    @B-McDonnell
B-McDonnell



    mentioned this issue
    
      Feb 2, 2025
    





  


  

      
   Merged

  







  









      

  
      



  

  @shenxianpeng





  


    

      

  

    

        
    
    

  


      
          
  
      
            Copy link

  

      
    

  


  

      

  
    Contributor


      

  


  

    

  





      


        


  
    

      
          
Right. The source here: https://github.com/commit-check/commit-check/blame/91ee12b3f5045523cbdacd07390c0ce98f3b8231/commit_check/main.py#L109-L124 indeed looks like an obvious bug. Thanks for reporting and PR!

      		
    

  





        


            

            
            

              
            

        

      


      

            
  

    
  
    
    

  

  


    












      

  
            

    











  
  

    

      

  





  




  
  

              

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


  



  






  
                  




    

  


      
  

    Assignees
  



        

    No one assigned







      


  


  

    Labels
  



    

      

    bug

  Something isn't working








      

  

    

        

    Projects
  


        




    None yet




  



      


  

    
  

    Milestone
  


      No milestone





    
      
          


  

    

      
        

          
    

    Development
  




            
  
Successfully merging a pull request may close this issue.





    


  
          
      fix: Ensure all checks pass without overwriting previous failures

    
      B-McDonnell/commit-check
    





      
    

  




      

    

  
      

  

    

      2 participants
    

    

        
          @shenxianpeng 
        
          @B-McDonnell