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

Add cassettes and modify testing to enable testing without internet #443

Closed
wants to merge 1 commit into from
Closed

Conversation

ajakk
Copy link

@ajakk ajakk commented Mar 30, 2020

This is unfinished and will still be modified a bit, but it is a good proof-of-concept for what I'm trying to do. tests/conftest.py has a pytest fixture in it which automatically handles logic related to vcrpy. Simply adding the argument vcr to any function will enable vcrpy for the function and will use/create the cassette with the function name in the cassette directory. With test classes, it is slightly more complex to use fixtures in class methods, and for this reason I added a class called FixtureInjector using an example from pytest docs. Further, this simplifies the use of vcr cassettes by not requiring function decorators with the correct path for every function you want to use cassettes for, which results in less work to use vcrpy.

Excluding the cassette directory, this commit currently adds 49 lines and removes 15 across tests/conftest.py and tests/test_buku{,Db}.py. I did some execution time testing as well by executing the command pytest tests/test_buku.py tests/test_bukuDb.py on the HEAD commit of both my and your repositories, 5 times each. Using the time returned by pytest, the average for your repository was 275.29 seconds, and the average in my repository was 59.19 seconds. In other words, using cassettes for the functions in these two test files sped up the test execution time by several minutes on my machine. This variability is completely attributed to the lack of network-related hangs during testing using the modifications in my repository.

Please keep in mind this is a work in progress and I still have some cleaning up to do with it, should you be interested in these changes.

Also adjust certain tests to use cassettes
@ajakk
Copy link
Author

ajakk commented Mar 30, 2020

I see CI doesn't like the long filenames. Truncating them wouldn't be hard to implement.

@jarun
Copy link
Owner

jarun commented Apr 3, 2020

We discussed that we can't allow so many new files. Are you planning to rework that?

@ajakk
Copy link
Author

ajakk commented Apr 5, 2020

If it were a single file you'd never be able to re-record a single test on its own. That's a maintenance headache in a way multiple files aren't, and there's no reason for it. Materially, what is the problem with multiple files? Maybe I can mitigate your concern.

@jarun
Copy link
Owner

jarun commented Apr 5, 2020

what is the problem with multiple files?

We discussed this already. I do not want to make the project littered with unnecessary files just for 1 platform. I appreciate your effort but this is just something I do not want to do. Thanks for your understanding.

@ajakk
Copy link
Author

ajakk commented Apr 5, 2020 via email

@jarun jarun closed this Apr 8, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Jun 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants