Test Runner Not Detecting Equal Lists of Strings

[cmoesel]

When running the test CqlNullologicalOperatorsTest:Coalesce:CoalesceFirstList, the test runner reports:

Test CqlNullologicalOperatorsTest:Coalesce:CoalesceFirstList status: fail expected: {'a'} actual: a

My engine is returning the following result:

{
  "resourceType": "Parameters",
  "parameters": [
    {
      "extension": [
        {
          "url": "http://hl7.org/fhir/StructureDefinition/cqf-cqlType",
          "valueString": "List<System.String>"
        }
      ],
      "name": "return",
      "valueString": "a"
    }
  ]
}

According to the CQL IG FHIR type mapping, System.String maps to FHIR.string. According to the $cql operation, the parameter should be repeated for each element in the list (but in this case there is only one). So... I believe my engine is returning the correct result to represent { 'a' }.

[cmoesel]

Note: This may be the same issue as #80 if there is just a general flaw with testing equality of lists. I've also noticed problems with lists of DateTimes, times, and non-Date intervals.

[brynrhodes]

Agreed, fix the runner's list comparison logic

[preston]

As I understand the spec for the return of $cql, "If the result is a list, the parameter will be repeated for each element in the list, with the special case that an empty list is represented with a parameter with a cqf-cqlType extension indicating the type of the result (e.g. List) and the cqf-isEmptyList extension" ... which does not define how to tell in

cql-tests-runner/src/extractors/value-map.ts

Line 16 in f2f940c

if (value.length === 1) {

whether or not a single item is supposed to be in a list.

I think we can deal with this by inferring from the "expected" return value, but it seems like the spec should clarify that an extension should be used, or something along those lines.

[cmoesel]

My implementation for the JS cql-execution engine always returns the cqf-cqlType extension (see above for example w/ single-item list). That way it's always clear what I'm sending.

Requiring the type would provide the most explicit verification that the engine is doing what is expected, but that does add some complexity to the engine implementation (for example, the JS engine doesn't always track types all the way through execution, so I'm pulling the resultType from the ELM to set the cqf-cqlResult extension).

Taking a hint from the expected expression is the most implementation-friendly, although it gives the implementation the benefit of the doubt in the single-item list cases. If the engine were to mistakenly return a single item not in a list, it would still pass.

So... I'm not sure what the right answer is, but to ensure the integrity of the system and be able to fully trust the results, I think I would lean toward making implementations send some indication that the result is a list (probably via the cqf-cqlType extension).

NOTE: I did have a little trouble wrapping my head around the placement of the extension, in that it feels a little weird to declare a List<...> type on every item in the list. E.g., { 'a', 'b' } is:

{
  "resourceType": "Parameters",
  "parameters": [{
    "extension": [{
      "url": "http://hl7.org/fhir/StructureDefinition/cqf-cqlType",
      "valueString": "List<System.String>"
    }],
    "name": "return",
    "valueString": "a"
  }, {
    "extension": [{
      "url": "http://hl7.org/fhir/StructureDefinition/cqf-cqlType",
      "valueString": "List<System.String>"
    }],
    "name": "return",
    "valueString": "b"
  }]
}

Part of me really wanted to put a top-level List<System.String> type higher (like on the Parameters resource itself), and declare each of the items as a System.String (or put the list type on the parameter but put the list items into part with their respective types). The example for { { 1, 2, 3 }, { 4, 5, 6 } }, however, clearly shows that each item in the list gets the full List<...> specifier (which in this example is extra fun due to the nested lists). I think it makes sense if you think about it as declaring the type for the return parameter (not that specific repeat of the return parameter), but it still puts us in a position where sometimes the type reflects the value[x] directly and sometimes it reflects the collection of value[x]s.