Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions src/main/java/org/eolang/lints/LtTestComment.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2025 Objectionary.com
* SPDX-License-Identifier: MIT
*/
package org.eolang.lints;

import com.github.lombrozo.xnav.Xnav;
import com.jcabi.xml.XML;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.stream.Collectors;
import org.eolang.parser.OnDefault;

/**
* Lint that warns if a comment is present at a test object.
* @since 0.0.59
*/
final class LtTestComment implements Lint<XML> {
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.

@kislayykumar looks like we can have this lint in XSL, there are no restrictions to implement it in Java, I believe. WDYT?

@Override
public Collection<Defect> defects(final XML xmir) throws IOException {
final Collection<Defect> defects = new ArrayList<>(0);
final Xnav xml = new Xnav(xmir.inner());
final List<Xnav> objects = xml
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.

@kislayykumar this variable seems to be redundant, we can inlline it in the forEach loop

.path("/object//o[@name and starts-with(@name, '+')]")
.collect(Collectors.toList());
Comment thread
kislayykumar marked this conversation as resolved.
for (final Xnav object : objects) {
final List<Xnav> comments = object
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.

@kislayykumar redundant variable, we should inline it in the if. Also, read this, please: https://www.yegor256.com/2015/09/01/redundant-variables-are-evil.html

.path("meta[@key='comment']")
.collect(Collectors.toList());
Comment thread
kislayykumar marked this conversation as resolved.
if (!comments.isEmpty()) {
defects.add(
new Defect.Default(
"test-has-comment",
Severity.WARNING,
new OnDefault(xmir).get(),
Integer.parseInt(object.attribute("line").text().orElse("0")),
"Test object contains a comment. Prefer short, self-explanatory test names instead of documenting them."
)
Comment thread
kislayykumar marked this conversation as resolved.
);
}
}
return defects;
}

@Override
public String motive() throws IOException {
return "Comments in test objects are discouraged as they often duplicate the test name. Prefer short, clear test names.";
}

@Override
public String name() {
return "test-has-comment";
}
}

62 changes: 62 additions & 0 deletions src/test/java/org/eolang/lints/LtTestCommentTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package org.eolang.lints;
Comment thread
kislayykumar marked this conversation as resolved.

import com.github.lombrozo.xnav.Xnav;
import com.jcabi.xml.XML;
import com.jcabi.xml.XMLDocument;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Test;

import java.io.IOException;
import java.util.Collection;

final class LtTestCommentTest {
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.

@kislayykumar as @coderabbitai outlined, let's introduce test that would check how lint works for tests that have comments above:

[] > foo
  # This test checks how my object works:
  [] +> calculates-critical-path

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@h1alexbel I'll add a test case that verifies the lint correctly detects comments in test objects. Let me generate the test method that uses your EO example:

@Test
void detectsCommentInTestObject() throws IOException {
    final XML xml = new XMLDocument(
        String.join(
            "",
            "<?xml version='1.0' encoding='UTF-8'?>",
            "<program>",
            "<objects>",
            "<o abstract='true' line='1' name='foo'>",
            "<o line='3' name='+calculates-critical-path'>",
            "<meta key='comment' value='This test checks how my object works:'/>",
            "</o>",
            "</o>",
            "</objects>",
            "</program>"
        )
    );
    final Collection<Defect> defects = new LtTestComment().defects(xml);
    MatcherAssert.assertThat(
        "Should detect comment in test object",
        defects,
        Matchers.hasSize(1)
    );
    final Defect defect = defects.iterator().next();
    MatcherAssert.assertThat(
        "Defect should have correct ID",
        defect.name(),
        Matchers.equalTo("test-has-comment")
    );
    MatcherAssert.assertThat(
        "Defect should have correct message",
        defect.message(),
        Matchers.equalTo("Test object contains a comment. Prefer self-explanatory test names.")
    );
}

This test case:

  1. Creates XML representing your EO code example with a test object +calculates-critical-path that has a comment
  2. Verifies that LtTestComment.defects() finds exactly one defect
  3. Checks that the defect has the correct ID and message as expected from the lint implementation

The XML structure follows the XMIR format where the comment from EO code gets converted to a <meta key='comment' value='...'/> element, which is what the LtTestComment lint searches for.


@Test
void returnsNoDefectWhenTestObjectHasNoComment() throws IOException {
XML xml = new XMLDocument(
"<program>" +
"<object name='foo' line='10'>" +
"<o name='+test' line='12'>" +
"</o>" +
"</object>" +
"</program>"
);
LtTestComment lint = new LtTestComment();
Collection<Defect> defects = lint.defects(xml);
MatcherAssert.assertThat(defects, Matchers.empty());
}
Comment thread
kislayykumar marked this conversation as resolved.

@Test
void returnsNoDefectWhenNoTestObjectPresent() throws IOException {
XML xml = new XMLDocument(
"<program>" +
"<object name='foo' line='10'>" +
"<o name='bar' line='12'>" +
"</o>" +
"</object>" +
"</program>"
);
LtTestComment lint = new LtTestComment();
Collection<Defect> defects = lint.defects(xml);
MatcherAssert.assertThat(defects, Matchers.empty());
}

@Test
void motiveReturnsExpectedString() throws IOException {
LtTestComment lint = new LtTestComment();
MatcherAssert.assertThat(
lint.motive(),
Matchers.containsString("Comments in test objects are discouraged")
);
}

@Test
void nameReturnsExpectedValue() {
LtTestComment lint = new LtTestComment();
MatcherAssert.assertThat(
lint.name(),
Matchers.equalTo("test-has-comment")
);
}
}
Comment thread
kislayykumar marked this conversation as resolved.
Loading