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.

Sign up for GitHub

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

Fix for #3189 - fixed ignored testcases count #3190

Merged
merged 2 commits into from
Nov 22, 2024
Merged

Fix for #3189 - fixed ignored testcases count #3190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
36c0ebd
Fix for #3189 - fixed ignored testcases count
AnTopch Nov 21, 2024
fb83289
Fix for #3189 - fixed ignored testcases count
AnTopch Nov 21, 2024
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
1 change: 1 addition & 0 deletions CHANGES.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ Fixed: GITHUB-3028: Execution stalls when using "use-global-thread-pool" (Krishn
Fixed: GITHUB-3122: Update JCommander to 1.83 (Antoine Dessaigne)
Fixed: GITHUB-3135: assertEquals on arrays - Failure message is missing information about the array index when an array element is unexpectedly null or non-null (Albert Choi)
Fixed: GITHUB-3140: assertEqualsDeep on Sets - Deep comparison was using the wrong expected value
Fixed: GITHUB-3189: Incorrect number of ignored tests displayed in the XML results

7.10.2
Fixed: GITHUB-3117: ListenerComparator doesn't work (Krishnan Mahadevan)
Expand Down
2 changes: 1 addition & 1 deletion testng-core/src/main/java/org/testng/reporters/AbstractXmlReporter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ public void add(Count count) {
this.failed += count.failed;
this.skipped += count.skipped;
this.retried += count.retried;
this.ignored += count.retried;
this.ignored += count.ignored;
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Add test coverage for ignored tests counting in XML reports

Based on the verification results:

  1. The JUnitReportsTest validates ignored count in JUnit reports but not for the XML reporter
  2. No specific test exists for the Count class's ignored tests accumulation
  3. The fix needs test coverage to prevent regression

Consider adding test coverage:

  • Add test case in CountTest.java to verify Count class's ignored tests accumulation
  • Add test case in AbstractXmlReporterTest to verify ignored tests are correctly reported in XML
🔗 Analysis chain

LGTM! This fix correctly addresses the ignored tests counting issue.

The change fixes issue #3189 by correctly accumulating the ignored count instead of erroneously using the retried count. This ensures that the XML report accurately reflects the number of ignored tests.

Consider adding a clarifying comment to document this fix:

  public void add(Count count) {
    this.passed += count.passed;
    this.failed += count.failed;
    this.skipped += count.skipped;
    this.retried += count.retried;
+   // Fixed in PR #3190: Use ignored count instead of retried count to fix incorrect reporting
    this.ignored += count.ignored;
  }

Let's verify that test coverage exists for this fix:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for test coverage of the ignored tests counting functionality

# Look for test files that verify ignored test counting
rg -l "ignored.*count|count.*ignored" --type java test/

Length of output: 103

Script:

#!/bin/bash
# Let's first find the test directory structure
fd -t d "test"

# Then search for test files related to XML reporting or Count class
rg -l "ignored.*count|count.*ignored" --type java

Length of output: 2838

Script:

#!/bin/bash
# Let's examine the JUnitReportsTest.java to see if it tests ignored count
rg -A 10 -B 10 "ignored.*count|count.*ignored" testng-core/src/test/java/test/junitreports/JUnitReportsTest.java

# Let's also check for any test files that might test the Count class directly
rg -l "class.*Count.*Test|test.*Count" --type java testng-core/src/test/

Length of output: 5981

Script:

#!/bin/bash
# Let's examine CountTest.java which might be directly testing the Count class
rg -A 10 -B 10 "class CountTest" testng-core/src/test/java/test/CountTest.java

# Also check if there are any existing tests for ignored tests in XML reports
ast-grep --pattern 'class $_ {
  $$$
  @Test
  $$$
  void $_($$) {
    $$$
    ignored$$$
    $$$
  }
  $$$
}'

Length of output: 870

}

private Count(Builder builder) {
Expand Down
11 changes: 11 additions & 0 deletions testng-core/src/test/java/test/reports/SimpleIgnoredSample.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package test.reports;

import org.testng.annotations.Ignore;
import org.testng.annotations.Test;

public class SimpleIgnoredSample {

@Test
@Ignore
public void ignored() {}
}
11 changes: 8 additions & 3 deletions testng-core/src/test/java/test/reports/XmlReporterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,12 @@ public void ensureReportGenerationWhenTestMethodIsWrappedWithWrappedTestNGMethod
@Test(description = "GITHUB-2886")
public void ensureConfigurationMethodsAreNotCountedAsSkippedInXmlReports() throws Exception {
File file =
runTest(RuntimeBehavior.FILE_NAME, null, JekyllTestSample.class, HydeTestSample.class);
runTest(
RuntimeBehavior.FILE_NAME,
null,
JekyllTestSample.class,
HydeTestSample.class,
SimpleIgnoredSample.class);
DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
DocumentBuilder builder = factory.newDocumentBuilder();
Document doc = builder.parse(file);
Expand All @@ -103,8 +108,8 @@ public void ensureConfigurationMethodsAreNotCountedAsSkippedInXmlReports() throw
int total = Integer.parseInt(node.getAttributes().getNamedItem("total").getNodeValue());
int passed = Integer.parseInt(node.getAttributes().getNamedItem("passed").getNodeValue());
int failed = Integer.parseInt(node.getAttributes().getNamedItem("failed").getNodeValue());
assertThat(ignored).isZero();
assertThat(total).isEqualTo(2);
assertThat(ignored).isEqualTo(1);
assertThat(total).isEqualTo(3);
Comment on lines +111 to +112
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

The assertions don't match the actual test implementation

The assertions expect a total of 3 tests with 1 ignored test, but SimpleIgnoredSample.java only contains a single test method that is ignored. This means the assertions assertThat(ignored).isEqualTo(1); assertThat(total).isEqualTo(3); are incorrect as they don't reflect the actual test class implementation.

  • SimpleIgnoredSample.java has only one test method ignored() which is annotated with both @Test and @Ignore
  • The total count should be 1 (not 3)
  • The ignored count is correctly set to 1
🔗 Analysis chain

Verify the SimpleIgnoredSample test class implementation.

The assertions expect exactly one ignored test and a total of 3 tests. Let's verify that SimpleIgnoredSample.class is properly implemented with one @Ignore annotated test method.

Let me gather more information about the SimpleIgnoredSample class implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify SimpleIgnoredSample class implementation

# Find and check the content of SimpleIgnoredSample class
echo "Searching for SimpleIgnoredSample class definition..."
rg -l "class SimpleIgnoredSample"

echo "Checking for @Ignore annotation usage..."
ast-grep --pattern 'class SimpleIgnoredSample {
  $$$
  @Ignore
  $$$
}'

Length of output: 382

Script:

#!/bin/bash
# Get the content of SimpleIgnoredSample class to verify test methods
echo "Content of SimpleIgnoredSample class:"
cat testng-core/src/test/java/test/reports/SimpleIgnoredSample.java

# Count total test methods
echo -e "\nCounting test methods..."
ast-grep --pattern 'class SimpleIgnoredSample {
  $$$
  @Test
  $$$
}'

# Count ignored test methods
echo -e "\nCounting ignored test methods..."
ast-grep --pattern 'class SimpleIgnoredSample {
  $$$
  @Ignore
  @Test
  $$$
}'

Length of output: 637

assertThat(passed).isEqualTo(2);
assertThat(failed).isZero();
}
Expand Down
Loading