dsp-api

dsp-api copied to clipboard

Pull Request Checklist

Task Description/Number

Move e2ez specs to separate SBT project

PR Type

• [ ] build/chore: maintenance tasks (no production code change)
• [ ] docs: documentation changes (no production code change)
• [ ] feat: represents new features
• [ ] fix: represents bug fixes
• [ ] perf: performance improvements
• [ ] refactor: represents production code refactoring
• [x] test: adding or refactoring tests (no production code change)
• [ ] deprecated: Deprecation warning (ideally referencing a migration guide)

Basic requirements for bug fixes and features

• [ ] Tests for the changes have been added
• [ ] Docs have been added / updated

Does this PR introduce a breaking change?

• [ ] Yes

Does this PR change client-test-data?

• [ ] Yes

Apr 03 '24 15:04 BalduinLandolt

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 13.13%. Comparing base (a622e4f) to head (ec7c581). Report is 1 commits behind head on main.

:exclamation: Current head ec7c581 differs from pull request most recent head d8e7125. Consider uploading reports for the commit d8e7125 to get more accurate results

@@            Coverage Diff             @@
##             main    #3166      +/-   ##
==========================================
- Coverage   13.22%   13.13%   -0.09%     
==========================================
  Files         270      266       -4     
  Lines       22244    22239       -5     
==========================================
- Hits         2941     2921      -20     
- Misses      19303    19318      +15     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Apr 03 '24 16:04 codecov[bot]

issue: Huge amount of duplication necessary for setting up the tests.

I would not have expected so many lines of code which we have to duplicate in order to have it in a separate module. TBH I think this is a bit too much to understand an maintain. Maybe we really have to consider creating a shared project or make e2e and integration dependent.

Apr 04 '24 11:04 seakayone

issue: Huge amount of duplication necessary for setting up the tests.

I would not have expected so many lines of code which we have to duplicate in order to have it in a separate module. TBH I think this is a bit too much to understand an maintain. Maybe we really have to consider creating a shared project or make e2e and integration dependent.

Personally, I don't feel it's really all that much.
And I invested quite a bit of effort in streamlining the existing setup, so it's not strictly duplicated, I would say it's largely improved (and surely more improvement could be done!).
I'm open to having a shared test project, but I would vote for merging this PR as is, and carefully extracting shared logic out into the shared project, simply because I wouldn't like to lose the streamlining efforts I put into it; but largely taking over the new code would probably break old tests in annoying ways.

If you don't want this merged as is, feel free to take the PR over and give it a shot. I'm not working on it before Monday.

Apr 04 '24 13:04 BalduinLandolt