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

Introduce caching of configured live accounts in each test process #3298

Merged
merged 3 commits into from May 9, 2022

Conversation

hpk42
Copy link
Contributor

@hpk42 hpk42 commented May 5, 2022

cache configured live account within a live test process, leading to a roughly 15% speed improvement on the overall test run as it removes an often multi-second overhead for the setup of live accounts from most tests. You can measure with "pytest --durations=10 tests/bench_test_setup.py".

This cache is only an in-process RAM dictionary for caching successfully configured live accounts from/to the filesystem.
#skip-changelog

@@ -13,6 +13,7 @@

import pytest
import requests
import py
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes but usage is minimal, and easy enough to get ported if needed. While i co-authored py, i didn't co-author pathlib :) and as we are not using in the app or in the binbdings, it's fine to keep using py.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(pytest has a dependency on py)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like pytest devs are constantly working on replacing py with pathlib everywhere: pytest-dev/pytest#7259
Once this work in finished, we'll have to port py usage to pathlib too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

k, i ported it. it's more code and it took me a bit to learn the pathlib API. Requires a bit more calls than what i like for this 2-3 line code thing. Also i think moving away from tmpdir/py.path.local is going to take quite a while anyway. Many of our tests use it and i wouldn't start the porting until it's really needed.

python/src/deltachat/testplugin.py Outdated Show resolved Hide resolved
python/src/deltachat/testplugin.py Show resolved Hide resolved
python/src/deltachat/testplugin.py Show resolved Hide resolved
@@ -13,6 +13,7 @@

import pytest
import requests
import py
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like pytest devs are constantly working on replacing py with pathlib everywhere: pytest-dev/pytest#7259
Once this work in finished, we'll have to port py usage to pathlib too.

python/src/deltachat/testplugin.py Show resolved Hide resolved
python/src/deltachat/testplugin.py Outdated Show resolved Hide resolved
return cachedict


def write_dict_to_dir(dic, target_dir):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just store a single file with JSON or pickle? This is like 1 lines instead of going through files in a directory:

from pathlib import Path
with Path(target_dir).open('w') as fp: json.dump(dic, fp)

And why store to files at all if nothing is persisted between pytest runs? Each time temporary directory is new and all stored dictionaries are lost.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just store a single file with JSON or pickle? This is like 1 lines instead of going through files in a directory:

from pathlib import Path
with Path(target_dir).open('w') as fp: json.dump(dic, fp)

There is no persistence for the caching, on purpose, and to keep it simple. Each testprocess just maintains an in-process cache so we don't need to care for outdated accounts (CI email accounts are valid for one hour).
Note that the code transfers a directory of files into a dict, and from a dict to a directory. It's a cheap way to record and replay a DC database dir with blob files and "shm" and "wal" files etc.

@hpk42 hpk42 merged commit cffa101 into master May 9, 2022
@hpk42 hpk42 deleted the accache branch May 9, 2022 19:22
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.

None yet

2 participants