search

eclipse / microprofile-metrics

microprofile-metrics
Apache License 2.0
100 stars 66 forks source link

Optional TCK in 2.3 uses non-standard REST endpoint parameters #601

Closed tjquinno closed 4 years ago

tjquinno commented 4 years ago

The optional portion of the metrics 2.3 TCK seems to use a non-standard JAX-RS feature: namely array parameters.

The MetricAppBeanOptional class in the TCK includes several methods with array parameters. Here is one example: https://github.com/eclipse/microprofile-metrics/blob/a1326c30d1be2018e1e7dfa177322f276d9739d6/tck/optional/src/main/java/org/eclipse/microprofile/metrics/test/optional/MetricAppBeanOptional.java#L171-L175):

Note the array parameter, something not mentioned in the JAX-RS spec and therefore outside the standard. This is distinct from collection-based parameters which are explicitly mentioned in the spec.

I get server-side errors when I run the TCK because the JAX-RS implementation we use (Jersey) does not have support for array parameters. But as a spec-compliant implementation, Jersey should be adequate for running the TCK, no?

I looked for a ParamConverter in the optional TCK code that might have handled these parameters but did not find one.

The TCK should not use constructs outside the JAX-RS spec, right?

jmartisk commented 4 years ago

I think you're right and these tests should probably be removed from the test suite. I'll come up with a PR and we'll see what others say

jmartisk commented 4 years ago

This also sounds like something we could backport to 2.3.x apart from just fixing it for 3.0

tjquinno commented 4 years ago

Or, rather than removing those tests, how about changing them to use collection-based parameters instead of array-based ones.

tjquinno commented 4 years ago

Any plans to make the same change in 2.3.x?

jmartisk commented 4 years ago

pr with backport https://github.com/eclipse/microprofile-metrics/pull/603