Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(taps): Add benchmark test on map transforms via pytest-benchmark #2011

Merged
merged 3 commits into from
Oct 10, 2023

Conversation

haleemur
Copy link
Contributor

@haleemur haleemur commented Oct 10, 2023

relates to #2002.

As suggested in #2002 (comment),

This PR adds a benchmark test using pytest-benchmark to test the performance of stream maps using simpleeval, and ensure that we do not harm application performance by using EvalWithCompoundTypes.

The PR does the following:

  • separate some logic into function _run_transform out of _test_transform to use for benchmarking
  • add a benchmark test for the repositories stream with stream size of 4000 records
  • add pytest-benchmark as a development dependency
  • add nox session for benchmarking (benches)

I'm less sure if I've implemented the following two correctly as I'm not familiar with github actions

  • add `benches to the github-actions workflow matrix
  • add action to upload benchmark results

📚 Documentation preview 📚: https://meltano-sdk--2011.org.readthedocs.build/en/2011/

@haleemur haleemur requested review from a team, edgarrmondragon and kgpayne as code owners October 10, 2023 02:21
@haleemur haleemur force-pushed the benchmark-stream-map branch 2 times, most recently from 6037938 to f541e64 Compare October 10, 2023 02:39
@haleemur haleemur force-pushed the benchmark-stream-map branch 2 times, most recently from 54c5175 to e8032dd Compare October 10, 2023 03:42
@haleemur
Copy link
Contributor Author

haleemur commented Oct 10, 2023

@edgarrmondragon I added the benchmark test, but having some trouble with the github action to post the benchmark result as a comment on the PR

for now, am removing the gh action to try to comment on the PR

@haleemur haleemur changed the title test(mappers): add benchmark test on map transforms via pytest-benchmark test(taps): add benchmark test on map transforms via pytest-benchmark Oct 10, 2023
@haleemur haleemur force-pushed the benchmark-stream-map branch from 22c3214 to 28da357 Compare October 10, 2023 19:44
@codecov
Copy link

codecov bot commented Oct 10, 2023

Codecov Report

Merging #2011 (28da357) into main (2dc27fc) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2011   +/-   ##
=======================================
  Coverage   87.42%   87.42%           
=======================================
  Files          59       59           
  Lines        5136     5136           
  Branches      830      830           
=======================================
  Hits         4490     4490           
  Misses        451      451           
  Partials      195      195           

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

@haleemur haleemur force-pushed the benchmark-stream-map branch from 95702e8 to 28da357 Compare October 10, 2023 20:09
@edgarrmondragon
Copy link
Collaborator

@edgarrmondragon I added the benchmark test, but having some trouble with the github action to post the benchmark result as a comment on the PR

for now, am removing the gh action to try to comment on the PR

Thank you so much for the contribution @haleemur! I think this is good enough as a first iteration. I think I can integrate Codspeed with very minor changes to comment in PRs.

@edgarrmondragon edgarrmondragon changed the title test(taps): add benchmark test on map transforms via pytest-benchmark test(taps): Add benchmark test on map transforms via pytest-benchmark Oct 10, 2023
@edgarrmondragon edgarrmondragon added this pull request to the merge queue Oct 10, 2023
Merged via the queue into meltano:main with commit 7675942 Oct 10, 2023
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants