Include test names for successful tests in verify command output

[mhanysz]

Observed Behavior

I want to run conftest verify --output junit in my CI pipeline and noticed that each test case in the output gets reported with the identical name. This leads to errors with tools that work with the generated junit XML, such as gitlab's pipeline tests view. Here's the output of the minimal example to reproduce the issue. Note the identical entries for both test cases:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
        <testsuite tests="2" failures="0" time="0.000" name="conftest.">
                <properties>
                        <property name="go.version" value="go1.19.5"></property>
                </properties>
                <testcase classname="conftest." name=".\policy\terraform\opentelekomcloud\issue_test.rego" time="0.000"></testcase>
                <testcase classname="conftest." name=".\policy\terraform\opentelekomcloud\issue_test.rego" time="0.000"></testcase>
        </testsuite>
</testsuites>

Expected Behavior

The test case entries in the junit xml generated when calling conftest verify --output junit should have unique names.

Steps to reproduce the Issue

1. Create file issue.rego with the following content (taken from website example):

package main

deny[msg] {
    proto := input.resource.aws_alb_listener[lb].protocol
    proto == "HTTP"
    msg = sprintf("ALB `%v` is using HTTP rather than HTTPS", [lb])
}

2. Create file issue_test.rego with the following content (taken from website example and slightly modified):

package main

test_deny_alb_http {
    cfg := parse_config("hcl2", `resource "aws_alb_listener" "lb_with_http" { protocol = "HTTP" }`)
    deny with input as cfg
}

test_deny_alb_https {
    cfg := parse_config("hcl2", `resource "aws_alb_listener" "lb_with_https" { protocol = "HTTPS" }`)
    count(deny) == 0 with input as cfg
}

3. Run conftest verify --output junit in the same folder (see observed behavior for output)

Configuration under Test

Conftest: v0.39.0 OPA: v0.49.0 OS: Reproduced on Win10 and Alpine Linux

[jalseth]

I believe this is the expected behavior when all tests succeed, at least with the current code. Compare to the JSON output and you'll note no rule names from the unit tests are emitted.

[
    {
        "filename": "issue_test.rego",
        "namespace": "",
        "successes": 1
    },
    {
        "filename": "issue_test.rego",
        "namespace": "",
        "successes": 1
    }
]

However, if you have failures, the test names are emitted. Using the example from the website:

test_deny_alb_http {
  cfg := parse_config("hcl2", `
    resource "aws_alb_listener" "lb_with_http" {
      protocol = "HTTP"
    }
  `)
  deny with input as cfg
}

test_deny_alb_https {
  cfg := parse_config("hcl2", `
    resource "aws_alb_listener" "lb_with_https" {
      protocol = "HTTPS"
    }
  `)
  not deny with input as cfg
}

test_deny_alb_protocol_unspecified {
  cfg := parse_config("hcl2", `
    resource "aws_alb_listener" "lb_with_unspecified_protocol" {
      foo = "bar"
    }
  `)
  not deny with input as cfg
}

We get:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
    <testsuite tests="3" failures="2" time="0.000" name="conftest.">
        <properties>
            <property name="go.version" value="go1.19.5"></property>
        </properties>
        <testcase classname="conftest." name="issue_test.rego" time="0.000"></testcase>
        <testcase classname="conftest." name="issue_test.rego - data.main.test_deny_alb_https" time="0.000">
            <failure message="Failed" type="">data.main.test_deny_alb_https</failure>
        </testcase>
        <testcase classname="conftest." name="issue_test.rego - data.main.test_deny_alb_protocol_unspecified" time="0.000">
            <failure message="Failed" type="">data.main.test_deny_alb_protocol_unspecified</failure>
        </testcase>
    </testsuite>
</testsuites>

You can see the successful test does not include the rule name, but the failures do. The JSON output mirrors this:

[
    {
        "filename": "issue_test.rego",
        "namespace": "",
        "successes": 1
    },
    {
        "filename": "issue_test.rego",
        "namespace": "",
        "successes": 0,
        "failures": [
            {
                "msg": "data.main.test_deny_alb_https"
            }
        ]
    },
    {
        "filename": "issue_test.rego",
        "namespace": "",
        "successes": 0,
        "failures": [
            {
                "msg": "data.main.test_deny_alb_protocol_unspecified"
            }
        ]
    }
]

I do not think this is a bug, but we can definitely consider this as an enhancement request.

[mhanysz]

The primary reason I reported this is that the omission of the rule names for successful tests limits the usefulness of the test report. The purpose of a test report is usually:

• reporting which tests failed, which works well with the current behavior
• trace how the test suite evolves over multiple test runs by seeing how the set of tests changes
• facilitate fixing failing tests by providing info about whether the failing test succeeded in the previous test run

Leaving out the rule names for successful tests impedes the latter two, because the reader of the report can't tell the successful tests apart.

The second issue I encountered with this behavior is that Gitlab doesn't handle junitxml files with identical test cases very well. It simply "deduplicates" them and only reports one successful test case, no matter how many are listed in the file. But that's a shortcoming of Gitlab in my opinion.

So even if the current behavior is as expected, I'd like to suggest to change it to also support the latter two purposes of test reports in general.

[jalseth]

The use case makes sense for the verify command where each rule in a *_test.rego file is testing a specific input for the deny rule, though not the test command for the same reasons in https://github.com/open-policy-agent/conftest/issues/731#issuecomment-1210150602. I've updated the title and switched the type to enhancement to more accurately reflect the request.

This would require updating the CheckResult struct to treat Successes similarly to the other result types.

[fredgate]

I have same problem as @mhanysz and agree with its remarks.

I actually find an interest in this for the verify command.
Note that for the stdout output (with report notes), the test names are clearly displayed. But the report parameter is allowed only for stdout ouput (so not junit).