Skip to content

Add benchmark for large table #70

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

Merged
merged 2 commits into from
Mar 26, 2022
Merged

Conversation

fcollonval
Copy link
Contributor

Preliminary work for addressing #68

@fcollonval
Copy link
Contributor Author

Small note: the new workflow won't be tested before being merged (as it needs to be on the default branch).

@fcollonval fcollonval marked this pull request as ready for review February 23, 2022 15:22
@martinRenou
Copy link
Collaborator

IIRC there is already end-to-end testing: See https://github.com/twosigma/beakerx_tabledisplay/blob/master/.github/workflows/python-package.yml#L135 and https://github.com/twosigma/beakerx_tests/blob/master/autotests/ci_tests.py

Would it be fine to replace those tests with Galata tests? @afshin

I guess we can also live with both beakerx_tests and galata for some time, too much testing is not so bad.

@afshin
Copy link

afshin commented Feb 24, 2022

I think benchmark/performance tests are the main thing that would help. The existing e2e tests are about functionality primarily, I think.

cc: @ildipo @DiegoAlbertoTorres

@fcollonval
Copy link
Contributor Author

Would it be fine to replace those tests with Galata tests?

I don't think this is worth the required investment (the existing tool is using another browser driver - aka selenium; so transition may not be easy). As @afshin pointed out, the existing e2e tests are targeting functionality. This introduces measurement with no target on functional testing; so no redundancy.

@ildipo ildipo merged commit 509bb38 into twosigma:master Mar 26, 2022
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.

4 participants