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

CommitValidationTest fails on revert commits #12766

Closed
stoyanK7 opened this issue Feb 23, 2023 · 4 comments · Fixed by #12778
Closed

CommitValidationTest fails on revert commits #12766

stoyanK7 opened this issue Feb 23, 2023 · 4 comments · Fixed by #12778

Comments

@stoyanK7
Copy link
Collaborator

Commit: 26cadbb
Message: Revert "doc: release notes for 10.8.0"
Pipeline fail: https://dev.azure.com/romanivanovjr/romanivanovjr/_build/results?buildId=13264&view=logs&j=c902ebb4-c9f8-5f09-4e17-ff78fbbc842e&t=9ca98c81-ff64-58f0-9d03-a23ac1c4a111&l=795

[ERROR] Failures: 
[ERROR]   CommitValidationTest.testCommitMessageHasProperStructure:183 Commit 26cadbb6772aa6ca85d6e95a9bff5d5366ca0430 message: "Revert "doc: release notes for 10.8.0"\n\nThis reverts commit ff8bcc73c3c22161656794c969bb28a8cb09595f.\n" is invalid
Proper commit message should adhere to the following rules:
    1) Must match one of the following patterns:
        ^Issue #\d+: .*$
        ^Pull #\d+: .*$
        ^(minor|config|infra|doc|spelling|dependency|supplemental): .*$
    2) It contains only one line of text
    3) Must not end with a period, space, or tab
    4) Commit message should be less than or equal to 200 characters

The rule broken was: 1
[INFO] 
[ERROR] Tests run: 3931, Failures: 1, Errors: 0, Skipped: 2

@romani
Copy link
Member

romani commented Feb 23, 2023

Reverts should be excluded from validation. I thought we already had support for this.

@stoyanK7
Copy link
Collaborator Author

On it,

@stoyanK7
Copy link
Collaborator Author

So the check regex would be ^Revert .*

Take for example the commit message

Revert "doc: release notes for 10.8.0"
This reverts commit ff8bcc73c3c22161656794c969bb28a8cb09595f.

and the code that checks it

private static int validateCommitMessage(String commitMessage) {
final String message = commitMessage.replace("\r", "").replace("\n", "");
final String trimRight = commitMessage.replaceAll("[\\r\\n]+$", "");
final int result;
if (!ACCEPTED_COMMIT_MESSAGE_PATTERN.matcher(message).matches()) {
// improper prefix
result = 1;
}
else if (!trimRight.equals(message)) {
// single-line of text (multiple new lines are allowed on end because of
// git (1 new line) and GitHub's web ui (2 new lines))
result = 2;
}
else if (INVALID_POSTFIX_PATTERN.matcher(message).matches()) {
// improper postfix
result = 3;
}
else if (message.length() > 200) {
// commit message has more than 200 characters
result = 4;
}
else {
result = 0;
}
return result;
}

Screenshot from 2023-02-26 19-14-12

Problem is that it violates second(else if (!trimRight.equals(message))) and third(else if (INVALID_POSTFIX_PATTERN.matcher(message).matches())) checks.

Shall we just this at the end of the file to work around all of this?

        if (message.matches(REVERT_COMMIT_MESSAGE_REGEX_PATTERN)) {
            result = 0;
        }

@romani
Copy link
Member

romani commented Feb 27, 2023

Yes, if it is revert commit, no more validation should be applied.

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

Successfully merging a pull request may close this issue.

2 participants