test: add tests/ folders to 5 modules missing CI coverage (#1040)#218
Open
emjay0921 wants to merge 3 commits into
Open
test: add tests/ folders to 5 modules missing CI coverage (#1040)#218emjay0921 wants to merge 3 commits into
emjay0921 wants to merge 3 commits into
Conversation
Five OpenSPP2 modules shipped without a tests/ folder, so CI's detect-changes coverage matrix had no flags to upload against them. This patch wires up at minimum one passing test per module so the matrix can start reporting coverage. - spp_drims_sl (data-only, SL locale): install + key hazard category + LKR currency activation. - spp_farmer_registry_dashboard (data-only, dashboard records): install + farmer dashboard category / group / overview records loaded. - spp_hide_menus_base (models): install + group_hide_menus_user seed + hide_menu/show_menu round-trip + idempotency + MENU_APP catalog well-formed. - spp_indicator_studio (UI bridge): install + indicator views + indicator category views + act_window points at spp.indicator. - spp_starter_farmer_registry (bundle): install + every declared dependency installed + config_smallholder_threshold seed loaded. Locally verified: spp_drims_sl reports 3/3 tests, spp_hide_menus_base reports 6/6. The other three fail to install in the local Docker setup only because they depend on modules outside OpenSPP2 (spp_dashboard_base lives in the private openspp-modules-v2 repo) — CI mounts both repos so those will install + run there. Out of scope per the ticket: - spp_registry — handled on a separate branch. - theme_openspp_muk — theme module, no Python logic to cover.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive unit tests and sanity checks across several OpenSPP Odoo modules, including spp_drims_sl, spp_farmer_registry_dashboard, spp_hide_menus_base, spp_indicator_studio, and spp_starter_farmer_registry. These tests verify successful module installation, seed data loading, and menu visibility transitions. The review feedback suggests improving test isolation in spp_hide_menus_base by creating a dedicated dummy menu instead of querying an arbitrary existing one, and removing redundant browse calls on already available recordsets.
Comment on lines
+22
to
+25
| # Pick any existing menu we can safely toggle in a test transaction. | ||
| cls.menu = cls.env["ir.ui.menu"].search([], limit=1) | ||
| if not cls.menu: | ||
| raise AssertionError("No ir.ui.menu records found to test against") |
There was a problem hiding this comment.
Searching for an arbitrary existing menu in the database can lead to flaky tests or unexpected failures if the selected menu already has restricted groups or other custom configurations. Creating a dedicated dummy menu for the test class ensures complete isolation and deterministic test behavior.
Suggested change
| # Pick any existing menu we can safely toggle in a test transaction. | |
| cls.menu = cls.env["ir.ui.menu"].search([], limit=1) | |
| if not cls.menu: | |
| raise AssertionError("No ir.ui.menu records found to test against") | |
| # Create a dummy menu to safely toggle in tests without relying on existing database records. | |
| cls.menu = cls.env["ir.ui.menu"].create({"name": "Test Menu"}) |
|
|
||
| def test_hide_menu_transition(self): | ||
| """hide_menu() flips state show → hide and snapshots original groups.""" | ||
| original_groups = self.env["ir.ui.menu"].browse(self.menu.id).group_ids |
There was a problem hiding this comment.
Since self.menu is already an ir.ui.menu recordset, calling self.env["ir.ui.menu"].browse(self.menu.id) is redundant. You can access group_ids directly from self.menu.
Suggested change
| original_groups = self.env["ir.ui.menu"].browse(self.menu.id).group_ids | |
| original_groups = self.menu.group_ids |
|
|
||
| def test_show_menu_restores_original_groups(self): | ||
| """show_menu() restores the snapshot taken at hide time.""" | ||
| original_groups = self.env["ir.ui.menu"].browse(self.menu.id).group_ids |
There was a problem hiding this comment.
Similarly, calling self.env["ir.ui.menu"].browse(self.menu.id) here is redundant. You can access group_ids directly from self.menu.
Suggested change
| original_groups = self.env["ir.ui.menu"].browse(self.menu.id).group_ids | |
| original_groups = self.menu.group_ids |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 19.0 #218 +/- ##
==========================================
+ Coverage 71.49% 71.50% +0.01%
==========================================
Files 993 1002 +9
Lines 58564 58627 +63
==========================================
+ Hits 41869 41921 +52
- Misses 16695 16706 +11
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
…g spp_dashboard_base in OpenSPP2 The dashboard module depends on spp_dashboard_base, which still lives in the private openspp-modules-v2 repo and isn't mounted in the OpenSPP2 CI runner. So 'odoo -i spp_farmer_registry_dashboard' fails in CI with "depends on module spp_dashboard_base. But the latter module is not available". Pulling the test scaffolding out of this PR so the rest of the OP#1040 tests can merge cleanly. spp_farmer_registry_dashboard tests can land in a follow-up once spp_dashboard_base is migrated to OpenSPP2 (or CI is updated to pull the private repo).
…walk Adds three more tests so the post-install hook on ir.module.module is exercised: - test_hide_menus_processes_catalog asserts the method walks MENU_APP and creates a state=hide spp.hide.menu record for every entry whose menu xml_id resolves in the test DB. - test_hide_menus_is_idempotent runs the method twice and asserts no duplicates / no state churn. - test_hide_menus_skips_unknown_modules confirms modules absent from MENU_APP (e.g. base) are left alone. Bumps the module's branch coverage from ~64% to >90%.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
4 OpenSPP2 modules shipped without a
tests/folder, so CI'sdetect-changescoverage matrix had no flags to upload against them. This wires up at minimum one passing test per module so the matrix can start reporting coverage.spp_drims_slspp_hide_menus_basespp_indicator_studiospp_starter_farmer_registryOut of scope per the ticket:
spp_registry— handled on a separate branch.theme_openspp_muk— theme module, no Python logic.Pulled out of this PR for a follow-up:
spp_farmer_registry_dashboard— depends onspp_dashboard_base, which still lives in the privateopenspp-modules-v2repo and isn't mounted in the OpenSPP2 CI runner, so the module can't install in CI. The test scaffolding can land in a follow-up oncespp_dashboard_baseis migrated to OpenSPP2 (or CI is updated to pull the private repo).Test plan
Locally verified the two modules whose dependency graph fits entirely inside
OpenSPP2:spp_drims_sl→0 failed, 0 error(s) of 3 testsspp_hide_menus_base→0 failed, 0 error(s) of 6 testsThe other two (
spp_indicator_studio,spp_starter_farmer_registry) hit a pre-existingread_groupissue inspp_hazardduring local install — CI doesn't trigger that path. Already passing on the previous CI run for both:spp_indicator_studioreports tests in CIspp_starter_farmer_registryreports tests in CICodecov:
Related links