Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
56 changes: 56 additions & 0 deletions src/main/java/org/eolang/lints/LtTestComment.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2016-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
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
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("//o[@name and starts-with(@name, '+')]")
.collect(Collectors.toList());
for (final Xnav object : objects) {
final List<Xnav> comments = object
Copy link
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']")
Copy link
Member

Choose a reason for hiding this comment

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

@kislayykumar In EO we have this:

[] > foo
  # Comment is here.
  [] +> my-test

After parsing, we have the following XMIR:

<object >
   <comments>
      <comment line="3">Comment is here</comment>
   </comments>
   <o line="1" name="foo" pos="0">
      <o base="ξ" line="1" name="xi🌵" pos="0"/>
      <o line="3" name="+my-test" pos="2">
         <o base="ξ" line="3" name="xi🌵" pos="2"/>
      </o>
   </o>
</object>

.collect(Collectors.toList());
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 self-explanatory test names."
)
);
}
}
return defects;
}

@Override
public String motive() throws IOException {
return "Avoid comments in test objects; use clear test names.";
Copy link
Member

Choose a reason for hiding this comment

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

@kislayykumar let's write more descriptive motive for lint. Please check src/main/resources/org/eolang/motives as reference for convention

}

@Override
public String name() {
return "test-has-comment";
}
}
3 changes: 2 additions & 1 deletion src/test/java/org/eolang/lints/LtByXslTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -359,7 +360,7 @@ void doesNotDuplicateDefectsWhenMultipleDefectsOnTheSameLine() throws Exception

@SuppressWarnings("StreamResourceLeak")
@Tag("deep")
@Timeout(180L)
@Timeout(value = 6, unit = TimeUnit.MINUTES)
@Test
void validatesEoPacksForErrors() throws IOException {
Files.walk(Paths.get("src/test/resources/org/eolang/lints/packs/single"))
Expand Down
70 changes: 70 additions & 0 deletions src/test/java/org/eolang/lints/LtTestCommentTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* SPDX-FileCopyrightText: Copyright (c) 2016-2025 Objectionary.com
* SPDX-License-Identifier: MIT
*/
package org.eolang.lints;

import com.jcabi.xml.XML;
import com.jcabi.xml.XMLDocument;
import java.io.IOException;
import java.util.Collection;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Test;

/**
* Test for {@link LtTestComment}.
*
* @since 0.0.1
*/
final class LtTestCommentTest {
Copy link
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

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 reportsNoDefectsWhenNoCommentPresent() throws IOException {
final XML xml = new XMLDocument(
Copy link
Member

Choose a reason for hiding this comment

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

@kislayykumar let's use org.eolang.parser.EoSyntax for constructing EO program

"<program><object name='foo' line='10'><o name='+test' line='12'></o></object></program>"
);
final LtTestComment lint = new LtTestComment();
Copy link
Member

Choose a reason for hiding this comment

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

@kislayykumar lint variable can be inlined

final Collection<Defect> defects = lint.defects(xml);
MatcherAssert.assertThat(
"Should not report defects when test object has no comment",
defects,
Matchers.empty()
);
}

@Test
void reportsNoDefectsWhenNoObjectPresent() throws IOException {
final XML xml = new XMLDocument(
"<program><object name='foo' line='10'><o name='bar' line='12'></o></object></program>"
);
final LtTestComment lint = new LtTestComment();
Copy link
Member

Choose a reason for hiding this comment

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

@kislayykumar lint variable can be inlined

final Collection<Defect> defects = lint.defects(xml);
MatcherAssert.assertThat(
"Should not report defects when no test object is present",
defects,
Matchers.empty()
);
}

@Test
void containsGuidanceInMotive() throws IOException {
final LtTestComment lint = new LtTestComment();
Copy link
Member

Choose a reason for hiding this comment

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

@kislayykumar lint variable can be inlined

MatcherAssert.assertThat(
"Motive must discourage comments in test objects",
lint.motive(),
Matchers.containsString("Avoid comments in test objects; use clear test names")
);
}

@Test
void returnsStableId() {
final LtTestComment lint = new LtTestComment();
Copy link
Member

Choose a reason for hiding this comment

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

@kislayykumar lint variable can be inlined

MatcherAssert.assertThat(
"Rule id must be 'test-has-comment'",
lint.name(),
Matchers.equalTo("test-has-comment")
);
}

}