feat(taps): Add checks for primary keys, replication keys and state partitioning keys to standard tap tests

[laurentS]

This PR adds 2 tests to the standard ones that all sdk-based taps run:

• 1 checks that PKeys are actually in the stream schema
• 1 does the same for replication keys

Without these tests, it is easy to write invalid streams that will only fail after a certain amount of processing (usually because the target will complain). See this thread in a separate PR for context.
These tests help catch coding errors much sooner, as some errors might live in rarely used streams (or untested ones).

Ideally we could add a third test that validates that all path variables are available in the stream context. This might be best addressed in a separate PR, as this is a bit harder to test, given the dynamic nature of the stream context.

[laurentS]

I've tested this with the code in MeltanoLabs/tap-gitlab#71 and it does correctly flag problems.

[codecov]

Codecov Report

Merging #829 (fce9ca1) into main (797226a) will decrease coverage by 0.01%.
The diff coverage is 79.16%.

@@            Coverage Diff             @@
##             main     #829      +/-   ##
==========================================
- Coverage   80.37%   80.36%   -0.02%     
==========================================
  Files          34       34              
  Lines        3425     3448      +23     
  Branches      681      687       +6     
==========================================
+ Hits         2753     2771      +18     
- Misses        499      502       +3     
- Partials      173      175       +2     

Impacted Files	Coverage Δ
singer_sdk/testing.py	93.68% <79.16%> (-4.93%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[visch]

@laurentS another trick I've been doing is baking a jsonl test into the github actions test. Requires a full data pull but 🤷 just an idea not really for this PR but I happened to see this while cruising through things today!

  jsonl_test:
    runs-on: ubuntu-latest
    steps:
    - uses: actions/checkout@v2
    - name: Set up Python
      uses: actions/setup-python@v2
      with:
        python-version: "3.8"
    - name: Install dependencies
      run: |
        python -m pip install --upgrade pip
        pip install meltano
        meltano install
    - name: Schema test
      run: |
        meltano run tap-name target-jsonl

No real reason we couldn't do a schema check in pytest instead of using a seperate target 🤷

Found the issue #227 I"ll update it! Sorry about hijacking this :D great PR

[edgarrmondragon]

Hi and thank you @laurentS! I see in the PR description that is meant to add tests for replication keys, but I only see them for primary keys and state partitioning keys. Let me know if you're planning to add them. Otherwise I can review (and merge) it as is 😄.

[laurentS]

Hi and thank you @laurentS! I see in the PR description that is meant to add tests for replication keys, but I only see them for primary keys and state partitioning keys. Let me know if you're planning to add them. Otherwise I can review (and merge) it as is 😄.

Sorry, this was my mistake, but I added an extra test, since it seems like a pretty useful one for correctness. I think this is ready for review now. There are some failures in CI that I'm not sure how to fix.

[edgarrmondragon]

There are some failures in CI that I'm not sure how to fix.

@laurentS Some of those are linter errors that I've suggested fixes for. The coverage failures I think we can ignore.

[edgarrmondragon]

pre-commit.ci autofix

[edgarrmondragon]

@laurentS codecov is not deal breaker since at least replication key checks should be covered by external GitLab tests when this lands on main. The state partitioning keys test requires a sample that uses those, so that's not on you really.

cc @aaronsteers in case you want to groom the title