-
Notifications
You must be signed in to change notification settings - Fork 19
Added lint for fix of comments in test code duplicate test names #752
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
base: master
Are you sure you want to change the base?
Changes from 5 commits
d7bef00
41d151e
f27bdef
6cc0089
894f58a
3240018
0b983e9
0ce483d
08a85aa
a4ee78a
330e547
f0f72b8
9ddbad4
cc9e8b4
306eb56
b53626c
6ed878b
0cf41ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| /* | ||
| * 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> { | ||
| @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 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| .path("//o[@name and starts-with(@name, '+')]") | ||
| .collect(Collectors.toList()); | ||
|
kislayykumar marked this conversation as resolved.
|
||
| for (final Xnav object : objects) { | ||
| final List<Xnav> comments = object | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kislayykumar redundant variable, we should inline it in the |
||
| .path(".//meta[@key='comment']") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kislayykumar In EO we have this: 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()); | ||
|
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." | ||
| ) | ||
|
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"; | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| /* | ||
|
|
||
| SPDX-FileCopyrightText: Copyright (c) 2016-2025 Objectionary.com | ||
| SPDX-License-Identifier: MIT | ||
| */ | ||
| package org.eolang.lints; | ||
|
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 { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kislayykumar as There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
The XML structure follows the XMIR format where the comment from EO code gets converted to a |
||
|
|
||
| @Test | ||
| void reportsNoDefectsWhenTestObjectHasNoComment() 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( | ||
| "Should not report defects when test object has no comment", | ||
| defects, | ||
| Matchers.empty() | ||
| ); | ||
| } | ||
|
kislayykumar marked this conversation as resolved.
|
||
|
|
||
| @Test | ||
| void reportsNoDefectsWhenNoTestObjectPresent() 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( | ||
| "Should not report defects when no test object is present", | ||
| defects, | ||
| Matchers.empty() | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void motiveContainsGuidance() throws IOException { | ||
| LtTestComment lint = new LtTestComment(); | ||
| MatcherAssert.assertThat( | ||
| "Motive must discourage comments in test objects", | ||
| lint.motive(), | ||
| Matchers.containsString("Comments in test objects are discouraged") | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void nameIsStableId() { | ||
| LtTestComment lint = new LtTestComment(); | ||
| MatcherAssert.assertThat( | ||
| "Rule id must be 'test-has-comment'", | ||
| lint.name(), | ||
| Matchers.equalTo("test-has-comment") | ||
| ); | ||
| } | ||
|
|
||
| @Test | ||
| void reportsDefectWhenTestObjectHasComment() throws IOException { | ||
| final XML xml = new XMLDocument( | ||
| "<program>" + | ||
| "<object name='foo' line='10'>" + | ||
| "<o name='+test' line='12'>" + | ||
| "<meta key='comment' line='13'/>" + | ||
| "</o>" + | ||
| "</object>" + | ||
| "</program>" | ||
| ); | ||
| final LtTestComment lint = new LtTestComment(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kislayykumar |
||
| final Collection<Defect> defects = lint.defects(xml); | ||
| MatcherAssert.assertThat( | ||
| "Should report exactly one defect when a test object has a comment", | ||
| defects, | ||
| Matchers.hasSize(1) | ||
| ); | ||
| } | ||
|
|
||
| } | ||
|
kislayykumar marked this conversation as resolved.
|
||
There was a problem hiding this comment.
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?