Conversation
There was a problem hiding this comment.
The pattern on Indico core is to have pytest.ini in the project root. Why not following the same pattern? If so, we would need to add the submodules (i.e., indico, plugins) to norecursedirs.
| @@ -0,0 +1,24 @@ | |||
| [pytest] | |||
| ; exclude unrelated folders | |||
| norecursedirs = | |||
There was a problem hiding this comment.
I see a few entries missing here, compared to Indico core, that may be interesting to have as well.
distetcnode_modules
| migrations | ||
| client |
There was a problem hiding this comment.
I guess that you have declared these without the indicorp/ prefix so that the file can be reused across repositories. This will, however, also ignore any migrations or client subdirectory, if they are ever added. Since we need to specify the name of the plugin, nonetheless in indico_plugins, I would prefer that we have a glob pattern that is explicit.
| migrations | |
| client | |
| indicorp/client | |
| indicorp/migrations |
| test-plugin: _check_plugin_pytest | ||
| uv run pytest -c plugins/$(plugin)/pytest.ini plugins/$(plugin) |
There was a problem hiding this comment.
Is this too much voodoo or is it divine intellect?
I'm not sure that we should be making assumptions about whether a plugin has pytest tests or not and, if it does, where the pytest.ini file might be located.
Each plugin directory is its own standalone project with its own architecture and QA decisions. This Makefile target breaks that isolation in ways that I'm not sure we should. If we did, then, why not also adding a target for building wheels, linting, another one for DB schema migrations, etc.?
The isolation that I speak of would still be justified for the building assets, as this one is necessary for setting up the plugin in the dev environment and relies on standard indico mechanisms.
| uvx maildump | ||
| ``` | ||
|
|
||
| ### Running tests |
There was a problem hiding this comment.
I believe that this section belongs in a CONTRIBUTING.md file.
|
|
||
| .PHONY: test-distro | ||
| test-distro: | ||
| uv run pytest -c $(DISTRO)/pytest.ini $(DISTRO) |
There was a problem hiding this comment.
If we keep pytest.ini in the root of the project, we can change this line by:
| uv run pytest -c $(DISTRO)/pytest.ini $(DISTRO) | |
| uv run pytest |
| .PHONY: test-core | ||
| test-core: | ||
| uv run pytest -c indico/pytest.ini indico |
There was a problem hiding this comment.
I have similar concerns with this target as I have with test-plugin (https://github.com/unconventionaldotdev/indicorp/pull/11/changes#r2782634229).
3 participants
No description provided.