search

weather-gov / weather.gov

weather.gov 2.0
Other
331 stars 10 forks source link

[Interop layer] test json schema validation #1848

Open jamestranovich-noaa opened 1 month ago

jamestranovich-noaa commented 1 month ago

Description

We have a few PHP tests that do very similar things and I believe they can all be combined in one test suite, since they all build a block (in PHP parlance) and verify the JSON schema against the output.

colinmurphy01 commented 1 month ago

@jamestranovich-noaa @greg-does-weather does this need to be included in this sprint in order to deploy to staging?

jamestranovich-noaa commented 1 month ago

No. I would consider this low priority and "nice to have" as we don't actually do anything meaningful with JSON validation (yet). The only reason to include this is to make sure we have test parity, but honestly I don't think this should be a blocker to interop deployment. If we get this in soon, then great! Otherwise I think we can wait. (Feel free to disagree @greg-does-weather )

greg-does-weather commented 1 month ago

Agreed, the schema tests are a way to ensure we know when the data structure changes so nothing accidentally changes, and nothing changes without being noted in the tests.

Instead of validating against the structures from the PHP blocks, maybe we should validate it against the JSONschema doc? I think the complexity with this one will be mocking all the API and database calls. I think we should be able to mock them using bundled data from the proxy, or alternatively (and maybe preferably?) we let this particular test actually make network calls. But in any case, I agree with James's assessment.

colinmurphy01 commented 1 month ago

Grouping with the other non-blocked interop issues in the Now priority and off the backlog

colinmurphy01 commented 2 weeks ago

Is this done?

jamestranovich-noaa commented 2 weeks ago

Nope, this is still a relatively low priority. CC @eric-gade

colinmurphy01 commented 1 week ago

Moving this back into backlog, unless @jamestranovich-noaa you planned on picking this up this sprint?

jamestranovich-noaa commented 1 week ago

Yes, let's punt on this one as it is still low priority compared to what else is on my plate (weather stories, properly handling interop restarts).