build-tools icon indicating copy to clipboard operation
build-tools copied to clipboard

Semantic convention generator: Clean up tests using separate input files

Open Oberon00 opened this issue 4 years ago • 1 comments

Many tests that use separate input files are not written very cleanly. The most important points:

  • The input YAML files contain lots of redundant stuff (copied from real semantic conventions). They should be minimized to only contain what is needed for the test.
  • The Python code of the tests could use some cleanup too similar to what was already done for some markdown tests, see https://github.com/open-telemetry/build-tools/pull/54/files#diff-6efabfcf3a7471d21e99e83685a2d0151cbb1404c52eed86f26a06d81ef17c44 where the test python code was cut down from 544 to 153 lines.
  • Instead of or in addition to the above point, try testing in such a way that we can use Python (triple-quoted) strings as input instead of needing separate files. This is not entirely trivial since file names do play a role (for generating links), but should nonetheless be doable. This would make the tests much, much more readable.

Further ideas, not as important:

  • Asserting on the line in which an exception occurs could be cleaner if we could somehow specify the expected exception in the input (e.g. with a YAML/HTML comment).

Oberon00 avatar Sep 08 '21 10:09 Oberon00

See also https://github.com/open-telemetry/build-tools/pull/79#discussion_r851360604:

These tests are not designed as integration tests, they are targeted feature tests and it makes no sense to drag in a full spec yaml. These tests should instead use a minimal yaml that just uses group foo with attribute bar instead of actual data. See https://github.com/open-telemetry/build-tools/issues/63.

Having tests that automatically pull in spec code would additionally make sense, but then we don't have e.g. a testMultipleEnum or testRef but instead a testGeneratorIsSpecCompatible (and you'd need to think about how to design the build/change process so that you can also change the output in "incompatible" ways, e.g. temporarily reference a spec PR branch that updates the spec so the semconv table check is in sync again).

Oberon00 avatar Apr 15 '22 16:04 Oberon00