Details
- Reviewers
ardumont vlorentz - Maniphest Tasks
- T1307: Remove mock storages used in tests.
- Commits
- rDLDPYb704652a86fb: tests: Use in-memory storage instead of mocking it
Diff Detail
- Repository
- rDLDPY PyPI loader
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Build has FAILED
Link to build: https://jenkins.softwareheritage.org/job/DLDPY/job/tox/12/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDPY/job/tox/12/console
Build has FAILED
Link to build: https://jenkins.softwareheritage.org/job/DLDPY/job/tox/14/
See console output for more information: https://jenkins.softwareheritage.org/job/DLDPY/job/tox/14/console
Jenkins build currently fails as a new swh-storage tag must be created and pushed.
You can actually tag and push if you need it.
The upload to pypi job should trigger itself when you push the tag.
I'm not sure about the associated debian package though. Maybe it triggers itself also?
swh/loader/pypi/tests/test_loader.py | ||
---|---|---|
68 | If you do this, you actually read the configuration from disk (so that can actually diverge from dev box if there is some locally). |
swh/loader/pypi/tests/test_loader.py | ||
---|---|---|
68 | Indeed you are right, we can have config loaded from files proceeding this way and welcome the side effects. What I am trying to achieve here is to load the default config without duplicating it in the tests code. I came with this other solution that loads it without reading config files on disk. from swh.core.config import ( merge_default_configs, read as config_read, SWH_DEFAULT_GLOBAL_CONFIG ) def parse_config_file(self, *args, **kwargs): default_conf = merge_default_configs(self.DEFAULT_CONFIG, self.ADDITIONAL_CONFIG, SWH_DEFAULT_GLOBAL_CONFIG) config = config_read(default_conf=default_conf) config['storage'] = {'cls': 'memory', 'args': {}} return config But apparently, the plan is to remove implicit configuration (T1410) and thus, based on my understanding, relies only on configuration files. So I am a little confused on what is the proper solution here. |
swh/loader/pypi/tests/test_loader.py | ||
---|---|---|
68 | Copy the default config to some global/constant, and return it |
swh/loader/pypi/tests/test_loader.py | ||
---|---|---|
68 | Yes, for now, do as proposed by vlorentz.
What i mean was, keep your override implementation of parse_config_file but remove the call to super().parse_config_file. Something like D770#inline-4166 would do.
I think the plan will be to remove implicit configuration by explicitely passing configuration along where it's needed. So that may simplify this anyway but now we will know where to look for changes ;) |
swh/loader/pypi/tests/test_loader.py | ||
---|---|---|
68 | All right, I will do as proposed. But I still do not like that configuration duplication, this can be a source of problem For instance, for the pypi loader, the configuration is the following: { 'cache': False, 'cache_dir': '', 'content_packet_size': 10000, 'content_packet_size_bytes': 104857600, 'content_size_limit': 104857600, 'debug': False, 'directory_packet_size': 25000, 'log_db': 'dbname=softwareheritage-log', 'occurrence_packet_size': 100000, 'release_packet_size': 100000, 'revision_packet_size': 100000, 'save_data': False, 'save_data_path': '', 'send_contents': True, 'send_directories': True, 'send_releases': True, 'send_revisions': True, 'send_snapshot': True, 'storage': {'args': {}, 'cls': 'memory'}, 'temp_directory': '/tmp/swh.loader.pypi/' } That's quite a lot of configuration keys, with some coming from the swh-core module |
swh/loader/pypi/tests/test_loader.py | ||
---|---|---|
68 | I agree. It's far from perfect. I do not like it either. It's a momentary trade-off. I like it better than the current side-effect of loading from disk. Note that the config from swh-core has not being changed for a while now (it still could but it has not for a long time, at least 2 years IIRC ;). |
Update:
- use a test configuration dict instead of calling super().parse_config_file
- bump required version of swh-storage and swh-loader-core
- use new test method names from swh-loader-core
Build is green
See https://jenkins.softwareheritage.org/job/DLDPY/job/tox/15/ for more details.