-
Notifications
You must be signed in to change notification settings - Fork 8
Require dependencies based on plugin config #213
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gazorby - happy to discuss any of the points I've raised.
ebb6f47
to
228049d
Compare
Thanks for rebasing this @gazorby :) I hope it wasn't too much of a pain. What do you think about doing SQLAlchemy as well? |
This allows me to keep up with the latest changes ;) Was thinking about it as well, but we need to fix #174 then |
e1d0a6f
to
da03830
Compare
2fc2497
to
386f9d6
Compare
Do you want this one in #245 too @peterschutt? |
No rush - we can make a new release any time. Can I do anything to help on this one? |
Ok, so we just miss sqla added to extras then. Are you happy with the |
386f9d6
to
3f10824
Compare
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
5d81b27
to
2b5d9c4
Compare
Hey @peterschutt, sqlalchemy has been added to optional dependencies and the |
This is a stack of work! Thanks so much for this! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @gazorby - thanks again, a lot of these comments are for my own notes - I'm going to start working through some updates for them.
The bigger the PR, the better it is if we keep the changes to bare minimum.
This prevents the need to reload any modules, albeit it is a bit slower.
Default values for `PluginConfig.do_cache` et al. are based on the `IS_XXX_INSTALLED` constants. We use `default_factory` so we can patch the module values in tests. Use `@validator` to test for missing dependency when gates are explicitly set.
b7aaee6
to
4ad9cb3
Compare
Import from within the test/fixture where appropriate.
54f790c
to
da3e4e6
Compare
* refactor: organise tests according to dependencies - organises tests according to their dependencies. - removes unnecessary autouse fixtures - removes gated imports at the module level - uses `--ignore` to exclude redis/sqlalchemy/sentry tests from no-extras tests * refactor: use `test_ignore_glob` pytest global This lets us skip test collection for a whole sub-package of tests if a dependency is unavailable. * refactor: log tests can run without sqlalchemy. This was only necessary due to the dependency on `tests.utils.controllers` in the `http_scope` fixture, which has now been removed. * refactor: don't need to skipif saq log tests As the log tests depend on `job` fixture which will automatically skip if SAQ not available. * refactor: remaining fixtures/tests requiring sqlalchemy moved into sqlalchemy directory.
Health checks are up.
Kudos, SonarCloud Quality Gate passed! |
Thanks heaps @gazorby ! |
* ♻️ refactor(deps): require dependencies based on plugin config * 🐛 fix: tox config * ♻️ refactor(tox): append coverage in testenv:noextras * ♻️ refactor(tox): don't run coverage in parallel mode in testenv:noextras * 🐛 fix: full coverage * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * ♻️ refactor: apply changes from review * 🐛 fix: linters * ♻️ refactor: full coverage * ♻️ refactor: update pytest plugin fixtures * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * ✅ test: fix tests * ♻️ refactor: add sqlalchemy to optional dependencies * ♻️ refactor: silent flake8 * ✅ test(tox): make noextras passing the full test suite * ♻️ refactor: silent pyright * 📝 docs: update readme * 📝 docs: update readme * refactor: requirements/tox/etc * style: rollback style changes The bigger the PR, the better it is if we keep the changes to bare minimum. * refactor: run pytest plugin tests in sub-process. This prevents the need to reload any modules, albeit it is a bit slower. * refactor: no top level conditional imports. * refactor: makes lifespan check imports and logic conditional * refactor: no implicit imports of modules with optional functionality * refactor: move any worker related stuff out of service module * refactor: default_factory and validator for dependency-based gates Default values for `PluginConfig.do_cache` et al. are based on the `IS_XXX_INSTALLED` constants. We use `default_factory` so we can patch the module values in tests. Use `@validator` to test for missing dependency when gates are explicitly set. * refactor: remove more module scoped conditional imports. Import from within the test/fixture where appropriate. * refactor: organise tests according to dependencies (#257) * refactor: organise tests according to dependencies - organises tests according to their dependencies. - removes unnecessary autouse fixtures - removes gated imports at the module level - uses `--ignore` to exclude redis/sqlalchemy/sentry tests from no-extras tests * refactor: use `test_ignore_glob` pytest global This lets us skip test collection for a whole sub-package of tests if a dependency is unavailable. * refactor: log tests can run without sqlalchemy. This was only necessary due to the dependency on `tests.utils.controllers` in the `http_scope` fixture, which has now been removed. * refactor: don't need to skipif saq log tests As the log tests depend on `job` fixture which will automatically skip if SAQ not available. * refactor: remaining fixtures/tests requiring sqlalchemy moved into sqlalchemy directory. * refactor: continue splitting tests up by required dependency. Health checks are up. * refactor: add method for setting lifecycle handlers. Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Peter Schutt <[email protected]>
Fixes #211