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.

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

JUnit testcase name not unique per failure #1608

Open
1 of 3 tasks
markafarrell opened this issue Nov 24, 2022 · 4 comments
Open
1 of 3 tasks

JUnit testcase name not unique per failure #1608

markafarrell opened this issue Nov 24, 2022 · 4 comments

Comments

@markafarrell
Copy link

Summary

Problem

When there are multiple linting issues with the same cause, the test case name is not unique.

This causes issues when the JUnit results are parsed (by GitLab) which discards all the failures except the final one.

image

Proposed Solution

Include the file and line number in the testcase name

e.g. <testcase classname="main.tf" name="terraform_deprecated_interpolation main.tf:197,31-75" time="0">

Command

docker run --rm -v $(pwd):/data -t ghcr.io/terraform-linters/tflint:v0.43.0 --format junit

Terraform Configuration

variable "bucket_name" {
  description = "Bucket name"
  type        = string
}
variable "bucket_name1" {
  description = "Bucket name"
  type        = string
}

terraform {
  required_providers {
    aws = {
      source  = "hashicorp/aws"
      version = ">= 2.7.0"
    }
  }
}

resource "aws_s3_bucket" "example" {
  bucket = "${var.bucket_name}"
}

resource "aws_s3_bucket" "example1" {
  bucket = "${var.bucket_name1}"
}

TFLint Configuration

Default

Output

<?xml version="1.0" encoding="UTF-8"?>
<testsuites>
  <testsuite tests="3" failures="3" time="0" name="">
    <properties></properties>
    <testcase classname="" name="terraform_required_version" time="0">
      <failure message=":0,0-0: terraform &#34;required_version&#34; attribute is required" type="Warning">Warning: terraform &#34;required_version&#34; attribute is required&#xA;Rule: terraform_required_version&#xA;Range: :0,0-0</failure>
    </testcase>
    <testcase classname="main.tf" name="terraform_deprecated_interpolation" time="0">
      <failure message="main.tf:20,12-32: Interpolation-only expressions are deprecated in Terraform v0.12.14" type="Warning">Warning: Interpolation-only expressions are deprecated in Terraform v0.12.14&#xA;Rule: terraform_deprecated_interpolation&#xA;Range: main.tf:20,12-32</failure>
    </testcase>
    <testcase classname="main.tf" name="terraform_deprecated_interpolation" time="0">
      <failure message="main.tf:24,12-33: Interpolation-only expressions are deprecated in Terraform v0.12.14" type="Warning">Warning: Interpolation-only expressions are deprecated in Terraform v0.12.14&#xA;Rule: terraform_deprecated_interpolation&#xA;Range: main.tf:24,12-33</failure>
    </testcase>
  </testsuite>

TFLint Version

0.43.0

Terraform Version

No response

Operating System

  • Linux
  • macOS
  • Windows
@bendrucker
Copy link
Member

I considered this, I didn't care for the idea of inserting ranges into the test names. The point of a test name is that it's identifiable across runs. For example, you could track flaky tests. A source range is not guaranteed to be stable.

This is GitLab's bug, they are making invalid parsing assumptions. A quick Google...

stoplightio/spectral#2025

Before we make things worse for everyone, please read through the linked GitLab thread and get a full understanding of the bug and whether there are any other workarounds.

Adding an incrementing number seems like an acceptable workaround, I can review a PR.

@markafarrell
Copy link
Author

@bendrucker One solution that might be a little more elegant is to use the terraform resource name as well as an incrementing number to give a little more context. I believe that should also stay stable across tests. Thoughts?

@bendrucker
Copy link
Member

Yeah, I thought about that but AFAICT it's impossible. There's no guarantee that a rule operates at the level of a specific resource. A rule could raise multiple issues for the same resource on different attributes. Several rules in the terraform ruleset operate on the terraform meta-block. One raises issues on comments. On all but the last you could theoretically generate a meaningful identifier but you have to implement this for every block type. A substantial effort requiring work outside the formatter package, all to work around the shortcomings of JUnit.

Ultimately the source range is the useful context and it's up to tools to make use of that. Despite its wide adoption JUnit is not full-featured and suffers from the fact that too much data is unstructured and stuffed into text. Contrast this with the GitHub Actions annotations system, which includes support for source positions:

https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#example-creating-an-annotation-for-an-error

Nothing we can do here is going to be "good" compared to that, so at least it should be simple.

@bendrucker
Copy link
Member

Improvements to the JUnit formatter are welcome, eg for some of the other extension fields GitLab seems to display. Part of the problem is that the only official looking spec I've found is very minimal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

2 participants