Skip to content

Issue #12847: Compare test violations without order - #21553

Open
rupeshkumar92a-arch wants to merge 1 commit into
checkstyle:masterfrom
rupeshkumar92a-arch:codex/issue-12847
Open

Issue #12847: Compare test violations without order#21553
rupeshkumar92a-arch wants to merge 1 commit into
checkstyle:masterfrom
rupeshkumar92a-arch:codex/issue-12847

Conversation

@rupeshkumar92a-arch

@rupeshkumar92a-arch rupeshkumar92a-arch commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Fixes #12847

AbstractModuleTestSupport previously compared expected and actual violations by list position. Tests with multiple violations on the same line therefore had to understand and reproduce the internal event ordering.

Compare the two collections through their existing violation-message patterns without requiring the lists to have the same order. The comparison remains one-to-one, so missing, unexpected, duplicate, wrong-line, and wrong-message violations still fail.

Validation:

  • Full test suites and coverage passed during ./mvnw -ntp clean verify.
  • ./mvnw -ntp verify -DskipTests passed on the final commit, including Checkstyle and PMD validation.
  • Main test suite: 6,810 tests, no failures or errors, two skipped.
  • Integration test suite: 1,358 tests, no failures, errors, or skips.
  • Eight focused matcher tests cover unordered violations, overlapping patterns, one-to-one matching, duplicate counts, and incorrect lines/messages.

@romani romani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

items:

.that(actualViolations.get(index))
.matches(testInputViolations.get(index).toRegex());
}
/* package */ static void verifyViolations(String file,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lets make it private.
test for this methods was good to looks at, but we should not preserve them.

@romani

romani commented Sep 13, 2026

Copy link
Copy Markdown
Member

backup of test that proved that new methods in Google truth are actually covers all:

///////////////////////////////////////////////////////////////////////////////////////////////
// checkstyle: Checks Java source code and other text files for adherence to a set of rules.
// Copyright (C) 2001-2026 the original author or authors.
//
// This library is free software; you can redistribute it and/or
// modify it under the terms of the GNU Lesser General Public
// License as published by the Free Software Foundation; either
// version 2.1 of the License, or (at your option) any later version.
//
// This library is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
// Lesser General Public License for more details.
//
// You should have received a copy of the GNU Lesser General Public
// License along with this library; if not, write to the Free Software
// Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
///////////////////////////////////////////////////////////////////////////////////////////////

package com.puppycrawl.tools.checkstyle;

import static com.puppycrawl.tools.checkstyle.internal.utils.TestUtil.getExpectedThrowable;

import java.util.List;

import org.junit.jupiter.api.Test;

import com.puppycrawl.tools.checkstyle.bdd.TestInputViolation;

public class ModuleTestSupportTest {

    @Test
    public void testViolationOrder() {
        AbstractModuleTestSupport.verifyViolations("input", List.of(
                new TestInputViolation(12, "second"),
                new TestInputViolation(2, "first"),
                new TestInputViolation(12, "first")),
                List.of("12:9: first message", "12:3: second message", "2: first message"));
    }

    @Test
    public void testOverlappingViolationPatterns() {
        AbstractModuleTestSupport.verifyViolations("input", List.of(
                new TestInputViolation(3, null),
                new TestInputViolation(3, "specific")),
                List.of("3:1: specific message", "3:8: other message"));
    }

    @Test
    public void testOverlappingPatternsRequireDistinctViolations() {
        getExpectedThrowable(AssertionError.class, () -> {
            AbstractModuleTestSupport.verifyViolations("input", List.of(
                    new TestInputViolation(3, null),
                    new TestInputViolation(3, "specific"),
                    new TestInputViolation(3, "specific")),
                    List.of("3:1: specific message", "3:8: other message",
                            "3:9: another message"));
        });
    }

    @Test
    public void testDuplicateViolations() {
        AbstractModuleTestSupport.verifyViolations("input", List.of(
                new TestInputViolation(3, "message"),
                new TestInputViolation(3, "message")),
                List.of("3:1: message", "3:8: message"));
    }

    @Test
    public void testMissingViolation() {
        getExpectedThrowable(AssertionError.class, () -> {
            AbstractModuleTestSupport.verifyViolations("input", List.of(
                    new TestInputViolation(3, "message"),
                    new TestInputViolation(3, "message")), List.of("3:1: message"));
        });
    }

    @Test
    public void testUnexpectedViolation() {
        getExpectedThrowable(AssertionError.class, () -> {
            AbstractModuleTestSupport.verifyViolations("input", List.of(
                    new TestInputViolation(3, "message")),
                    List.of("3:1: message", "3:8: message"));
        });
    }

    @Test
    public void testWrongMessage() {
        getExpectedThrowable(AssertionError.class, () -> {
            AbstractModuleTestSupport.verifyViolations("input", List.of(
                    new TestInputViolation(3, "expected")), List.of("3:1: different"));
        });
    }

    @Test
    public void testWrongLine() {
        getExpectedThrowable(AssertionError.class, () -> {
            AbstractModuleTestSupport.verifyViolations("input", List.of(
                    new TestInputViolation(3, "message")), List.of("4:1: message"));
        });
    }

}

@romani

romani commented Sep 13, 2026

Copy link
Copy Markdown
Member

@rupeshkumar92a-arch , please share maven output of test failures when:

  • extra violation comment is present in Input file
  • missed violation comment is in Input file, but present in expected array.
  • message is different in arrays and in trailing comment of Input file

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement sorting in AbstractModuleTestSupport#verifyViolations

2 participants