xanzy / go-gitlab

GitLab Go SDK
Apache License 2.0
2.39k stars 946 forks source link

Bug: Pipeline test cases system output has incorrect type #1757

Closed clawoflight closed 1 year ago

clawoflight commented 1 year ago

The pipeline test cases system output is currently typed as string. There was a PR to change it to a struct, which was reverted.

Those are actually not the only 2 types that can occur because GitLab supports multiple test frameworks with different formats:

It should be possible to handle these variants without crashing. Either ignoring errors when the type is not string, which would already be a useful improvement, or providing a variant type over the possibilities, or normalizing everything to strings (by accessing the corresponding struct member re. concatenating the strings in the array).

Apologies for (re) opening a can of worms, but this issue breaks a downstream project for my use case: https://github.com/mvisonneau/gitlab-ci-pipelines-exporter/issues/682

svanharmelen commented 1 year ago

I guess we should just make it an interface{} in that case, but can you share some pointers that show what flavors of system output Gitlab actually returns?

clawoflight commented 1 year ago

This is a redacted and truncated example of what we get in a Java project:

{
  "total_time": 1907.8749999999905,
  "total_count": 3418,
  "success_count": 2822,
  "failed_count": 558,
  "skipped_count": 38,
  "error_count": 0,
  "test_suites": [
    {
      "name": "unit test",
      "total_time": 1907.8749999999905,
      "total_count": 3418,
      "success_count": 2822,
      "failed_count": 558,
      "skipped_count": 38,
      "error_count": 0,
      "suite_error": null,
      "test_cases": [
        {
          "status": "failed",
          "name": "REDACTEDTest",
          "classname": "REDACTED",
          "file": null,
          "execution_time": 6.504,
          "system_output": "redacted multi-line string",
          "stack_trace": null,
          "recent_failures": null
        },
        {
          "status": "failed",
          "name": "REDACTEDTest2",
          "classname": "REDACTED",
          "file": null,
          "execution_time": 5.346,
          "system_output": [
            "redacted multi-line string",
            "redacted multi-line string"
          ],
          "stack_trace": null,
          "recent_failures": null
        },
      ]
    }
  ]
}

Additional observed values were null and objects - here is an example that was referenced in the PR I linked above:

{
      "name": "rspec-ee system pg11 geo",
      "failed_count": 41,
      "test_cases": [
        {
          "status": "failed",
          "name": "example",
          "classname": "ee.spec.features.geo_node_spec",
          "file": "./ee/spec/features/geo_node_spec.rb",
          "execution_time": 6.324748,
          "system_output": {
            "__content__": "\n",
            "message": "RSpec::Core::MultipleExceptionError",
            "type": "RSpec::Core::MultipleExceptionError"
          }
        }
      ]
    }

Let me know if I can help in any other way :)

svanharmelen commented 1 year ago

Thanks for you efforts, but this doesn't really show what GitLab can return. It only shows what is returned in your specific case. It would be good to know which types the API can return so we can check if we can support the types directly or just make the field an interface{}.

clawoflight commented 1 year ago

Well, it shows 4 cases, not just mine :) The json object is from an upstream GitLab test, not from my instance.

GitLab sadly does not document a schema for this field, in fact it documents very little for this response. The above are all the types which I have encountered in related threads on the internet so far.

svanharmelen commented 1 year ago

Sorry, I didn't look good enough I guess 😊 But in that case let's just make it an interface{} for now... Feel free to make a PR for that 😏

clawoflight commented 1 year ago

No worries!

It's been 5 years since I wrote a line of go, but I'll give it a try. Can you link me to a recent tooling cheatsheet and/or describe how I can run the tests?

svanharmelen commented 1 year ago

No worries, I just pushed a commit to update the field (a42af8593e45e5a55a6008e482ecbc3dc6f5d06b) so will close this issue and hope this solves the incompatibilities with this struct.

clawoflight commented 1 year ago

Thank you! Shouldn't the test data in https://github.com/xanzy/go-gitlab/blob/master/testdata/get_pipeline_testreport.json and the corresponding test(s) be adapted as well?

svanharmelen commented 1 year ago

Sure, updated in 7e3c23c9c221c0a334e11356503a99d41f206651

clawoflight commented 1 year ago

Thanks! I tried adding this locally, but didn't realize that I needed to use map[string]interface{} instead of map[string]string and the same for arrays. At least I know how to run the tests now :)

clawoflight commented 1 year ago

It would be great if you could ping me when the release containing this fix is tagged so I can ask downstream to update :)

svanharmelen commented 1 year ago

Tagged v0.88.0