search

singer-io / singer-python

Writes the Singer format from Python
https://singer.io
Apache License 2.0
538 stars 128 forks source link

Fix testing of get_standard_metadata #132

Closed judahrand closed 3 years ago

judahrand commented 3 years ago

Description of change

This Pull Request aims to fix the tests for metadata.get_standard_metadata which previously could not fail and would never detect any bugs due to two if clauses. Once these clauses were removed lots of other things in the test failed. I have fixed these to the best of my ability but obviously as I'm changing the test it would be good if someone could check that I have understood the expected behaviour correctly.

Manual QA steps

Risks

Rollback steps

judahrand commented 3 years ago

Should "inclusion": "available" always be included in the "breadcrumb": [] metadata? Even if there is no schema? I couldn't work out what the standard behaviour was expected to be. The tests and the actual function disagreed... I have currently changed the tests to stick to current behaviour so as to introduce minimum changes to the function itself.

Is this acceptable?

judahrand commented 3 years ago

@luandy64 Seems you've opened a new PR adding some additional changes so I'll close this one for neatness.

luandy64 commented 3 years ago

Yup, I wanted some things on top of the work you had done, so I added them. Thanks for catching this!