From 13ccd9cee4977028c91ef2d2f37e6730e5a88442 Mon Sep 17 00:00:00 2001 From: Victor Feyens Date: Tue, 29 Nov 2022 13:59:14 +0000 Subject: [PATCH] [IMP] test_lint: detect useless manifest content Keep the manifests as light as possible, to easily see custom behavior/content. Complete the work of previous commits cleaning the manifests content: * 42bad1a6d21e086e3bb766f9813d65902b3c232a * ef7005f52416912a9efab5ec2767432c4181fb4d and make sure this kind of cleanup commit is not necessary in the future because it is now automatically verified by a dedicated test. closes odoo/odoo#107735 Related: odoo/enterprise#34903 Signed-off-by: Julien Castiaux --- addons/base_address_extended/__manifest__.py | 2 +- addons/google_account/__manifest__.py | 1 - addons/hr_recruitment_skills/__manifest__.py | 1 - addons/http_routing/__manifest__.py | 2 +- addons/l10n_bg/__manifest__.py | 2 - addons/l10n_din5008/__manifest__.py | 1 - addons/l10n_eg_edi_eta/__manifest__.py | 1 - addons/l10n_gcc_invoice/__manifest__.py | 1 - addons/l10n_gcc_pos/__manifest__.py | 1 - addons/l10n_hk/__manifest__.py | 1 - addons/l10n_hr/__manifest__.py | 3 +- addons/l10n_it_edi/__manifest__.py | 1 - addons/l10n_ke/__manifest__.py | 1 - addons/l10n_latam_check/__manifest__.py | 2 - addons/l10n_ro/__manifest__.py | 2 +- addons/l10n_sa_pos/__manifest__.py | 1 - addons/l10n_si/__manifest__.py | 1 - addons/phone_validation/__manifest__.py | 2 +- addons/portal/__manifest__.py | 2 +- addons/pos_stripe/__manifest__.py | 1 - addons/spreadsheet_dashboard/__manifest__.py | 2 - .../website_sale_comparison/__manifest__.py | 1 - addons/website_sale_wishlist/__manifest__.py | 1 - odoo/addons/base/tests/test_module.py | 2 +- odoo/addons/test_lint/tests/test_manifests.py | 98 +++++++++++++++++-- odoo/modules/module.py | 2 +- 26 files changed, 99 insertions(+), 36 deletions(-) diff --git a/addons/base_address_extended/__manifest__.py b/addons/base_address_extended/__manifest__.py index 5d6f593ed15..45a9de7cff7 100644 --- a/addons/base_address_extended/__manifest__.py +++ b/addons/base_address_extended/__manifest__.py @@ -3,7 +3,7 @@ { 'name': 'Extended Addresses', 'summary': 'Add extra fields on addresses', - 'sequence': '19', + 'sequence': 19, 'version': '1.1', 'category': 'Hidden', 'description': """ diff --git a/addons/google_account/__manifest__.py b/addons/google_account/__manifest__.py index f1d75079efd..7fd61ddb6e5 100644 --- a/addons/google_account/__manifest__.py +++ b/addons/google_account/__manifest__.py @@ -9,6 +9,5 @@ The module adds google user in res user. ======================================== """, 'depends': ['base_setup'], - 'data': [], 'license': 'LGPL-3', } diff --git a/addons/hr_recruitment_skills/__manifest__.py b/addons/hr_recruitment_skills/__manifest__.py index ec1b49776ff..8a2786e8573 100644 --- a/addons/hr_recruitment_skills/__manifest__.py +++ b/addons/hr_recruitment_skills/__manifest__.py @@ -15,7 +15,6 @@ 'security/ir.model.access.csv', ], 'installable': True, - 'application': False, 'auto_install': True, 'license': 'LGPL-3', } diff --git a/addons/http_routing/__manifest__.py b/addons/http_routing/__manifest__.py index b562bf71ce1..965d7c204f4 100644 --- a/addons/http_routing/__manifest__.py +++ b/addons/http_routing/__manifest__.py @@ -3,7 +3,7 @@ { 'name': 'Web Routing', 'summary': 'Web Routing', - 'sequence': '9100', + 'sequence': 9100, 'category': 'Hidden', 'description': """ Proposes advanced routing options not available in web or base to keep diff --git a/addons/l10n_bg/__manifest__.py b/addons/l10n_bg/__manifest__.py index 550af99ddeb..30696f1028b 100644 --- a/addons/l10n_bg/__manifest__.py +++ b/addons/l10n_bg/__manifest__.py @@ -2,10 +2,8 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. { 'name': 'Bulgaria - Accounting', - 'icon': '/l10n_bg/static/description/icon.png', 'version': '1.0', 'category': 'Accounting/Localizations/Account Charts', - 'author': 'Odoo S.A.', 'description': """ Chart accounting and taxes for Bulgaria """, diff --git a/addons/l10n_din5008/__manifest__.py b/addons/l10n_din5008/__manifest__.py index 5a2547116aa..44a028edb4e 100644 --- a/addons/l10n_din5008/__manifest__.py +++ b/addons/l10n_din5008/__manifest__.py @@ -6,7 +6,6 @@ 'version': '1.0', 'category': 'Accounting/Localizations', 'description': "This is the base module that defines the DIN 5008 standard in Odoo.", - 'author': 'Odoo S.A.', 'depends': ['account'], 'data': [ 'report/din5008_report.xml', diff --git a/addons/l10n_eg_edi_eta/__manifest__.py b/addons/l10n_eg_edi_eta/__manifest__.py index d238cb0a4df..f5b17958db8 100644 --- a/addons/l10n_eg_edi_eta/__manifest__.py +++ b/addons/l10n_eg_edi_eta/__manifest__.py @@ -8,7 +8,6 @@ This module integrate with the ETA Portal to automatically sign and send your invoices to the tax Authority. Special thanks to Plementus for their help in developing this module. """, - 'author': 'Odoo S.A.', 'website': 'https://www.odoo.com', 'category': 'account', 'version': '0.1', diff --git a/addons/l10n_gcc_invoice/__manifest__.py b/addons/l10n_gcc_invoice/__manifest__.py index b3f78866f44..e6bef3bd92d 100644 --- a/addons/l10n_gcc_invoice/__manifest__.py +++ b/addons/l10n_gcc_invoice/__manifest__.py @@ -3,7 +3,6 @@ { 'name': 'G.C.C. - Arabic/English Invoice', 'version': '1.0.0', - 'author': 'Odoo S.A.', 'category': 'Accounting/Localizations', 'description': """ Arabic/English for GCC diff --git a/addons/l10n_gcc_pos/__manifest__.py b/addons/l10n_gcc_pos/__manifest__.py index 7082641dfb1..2148957737c 100644 --- a/addons/l10n_gcc_pos/__manifest__.py +++ b/addons/l10n_gcc_pos/__manifest__.py @@ -2,7 +2,6 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. { 'name': 'Gulf Cooperation Council - Point of Sale', - 'author': 'Odoo S.A.', 'category': 'Accounting/Localizations/Point of Sale', 'description': """ GCC POS Localization diff --git a/addons/l10n_hk/__manifest__.py b/addons/l10n_hk/__manifest__.py index f618a13a177..d125cb92104 100644 --- a/addons/l10n_hk/__manifest__.py +++ b/addons/l10n_hk/__manifest__.py @@ -6,7 +6,6 @@ 'version': '1.0', 'category': 'Accounting/Localizations/Account Charts', 'description': """ This is the base module to manage chart of accounting and localization for Hong Kong """, - 'author': 'Odoo S.A.', 'depends': ['account'], 'data': [ 'data/account_chart_template_data.xml', diff --git a/addons/l10n_hr/__manifest__.py b/addons/l10n_hr/__manifest__.py index f56b691ba08..c29130a294e 100644 --- a/addons/l10n_hr/__manifest__.py +++ b/addons/l10n_hr/__manifest__.py @@ -4,14 +4,13 @@ "name": "Croatia - Accounting", "description": """ Croatian Chart of Accounts updated (RRIF ver.2021) - + Sources: https://www.rrif.hr/dok/preuzimanje/Bilanca-2016.pdf https://www.rrif.hr/dok/preuzimanje/RRIF-RP2021.PDF https://www.rrif.hr/dok/preuzimanje/RRIF-RP2021-ENG.PDF """, "version": "13.0", - "author": "Odoo S.A.", 'category': 'Accounting/Localizations/Account Charts', 'depends': [ diff --git a/addons/l10n_it_edi/__manifest__.py b/addons/l10n_it_edi/__manifest__.py index 9e372c98ff5..d1ce718365a 100644 --- a/addons/l10n_it_edi/__manifest__.py +++ b/addons/l10n_it_edi/__manifest__.py @@ -13,7 +13,6 @@ 'account_edi_proxy_client', ], 'auto_install': ['l10n_it', 'account_edi'], - 'author': 'Odoo S.A.', 'description': """ E-invoice implementation """, diff --git a/addons/l10n_ke/__manifest__.py b/addons/l10n_ke/__manifest__.py index 03f11c74ad2..ff550da19bb 100644 --- a/addons/l10n_ke/__manifest__.py +++ b/addons/l10n_ke/__manifest__.py @@ -8,7 +8,6 @@ 'description': """ This provides a base chart of accounts and taxes template for use in Odoo. """, - 'author': 'Odoo S.A.', 'depends': [ 'account', ], diff --git a/addons/l10n_latam_check/__manifest__.py b/addons/l10n_latam_check/__manifest__.py index 3338b007f0e..4a5b5f990ad 100644 --- a/addons/l10n_latam_check/__manifest__.py +++ b/addons/l10n_latam_check/__manifest__.py @@ -55,6 +55,4 @@ There are 2 main Payment Methods additions: 'wizards/account_payment_register_views.xml', ], 'installable': True, - 'auto_install': False, - 'application': False, } diff --git a/addons/l10n_ro/__manifest__.py b/addons/l10n_ro/__manifest__.py index f26027d5ea2..8ab39ba2f12 100644 --- a/addons/l10n_ro/__manifest__.py +++ b/addons/l10n_ro/__manifest__.py @@ -10,7 +10,7 @@ { "name": "Romania - Accounting", - "author": ["Fekete Mihai (NextERP Romania SRL)", "Odoo S.A."], + "author": "Fekete Mihai (NextERP Romania SRL), Odoo S.A.", 'category': 'Accounting/Localizations/Account Charts', 'version': '1.0', "depends": [ diff --git a/addons/l10n_sa_pos/__manifest__.py b/addons/l10n_sa_pos/__manifest__.py index a34a93d8dd2..5405615cc00 100644 --- a/addons/l10n_sa_pos/__manifest__.py +++ b/addons/l10n_sa_pos/__manifest__.py @@ -2,7 +2,6 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. { 'name': 'Saudi Arabia - Point of Sale', - 'author': 'Odoo S.A.', 'category': 'Accounting/Localizations/Point of Sale', 'description': """ K.S.A. POS Localization diff --git a/addons/l10n_si/__manifest__.py b/addons/l10n_si/__manifest__.py index 30784cae0c8..2257efb4e19 100644 --- a/addons/l10n_si/__manifest__.py +++ b/addons/l10n_si/__manifest__.py @@ -4,7 +4,6 @@ { "name": "Slovenian - Accounting", "version": "1.1", - "author": "Odoo S.A.", "category": "Accounting/Localizations/Account Charts", "description": """ Chart of accounts and taxes for Slovenia. diff --git a/addons/phone_validation/__manifest__.py b/addons/phone_validation/__manifest__.py index 46e0ab7c030..a191e008431 100644 --- a/addons/phone_validation/__manifest__.py +++ b/addons/phone_validation/__manifest__.py @@ -5,7 +5,7 @@ 'name': 'Phone Numbers Validation', 'version': '2.1', 'summary': 'Validate and format phone numbers', - 'sequence': '9999', + 'sequence': 9999, 'category': 'Hidden', 'description': """ Phone Numbers Validation diff --git a/addons/portal/__manifest__.py b/addons/portal/__manifest__.py index aedcb33877f..eb9fb69df1c 100644 --- a/addons/portal/__manifest__.py +++ b/addons/portal/__manifest__.py @@ -4,7 +4,7 @@ { 'name': 'Customer Portal', 'summary': 'Customer Portal', - 'sequence': '9000', + 'sequence': 9000, 'category': 'Hidden', 'description': """ This module adds required base code for a fully integrated customer portal. diff --git a/addons/pos_stripe/__manifest__.py b/addons/pos_stripe/__manifest__.py index ef3e9a4d2ac..d6b135d4396 100644 --- a/addons/pos_stripe/__manifest__.py +++ b/addons/pos_stripe/__manifest__.py @@ -6,7 +6,6 @@ 'category': 'Sales/Point of Sale', 'sequence': 6, 'summary': 'Integrate your POS with a Stripe payment terminal', - 'description': '', 'data': [ 'views/pos_payment_method_views.xml', 'views/assets_stripe.xml', diff --git a/addons/spreadsheet_dashboard/__manifest__.py b/addons/spreadsheet_dashboard/__manifest__.py index 44f2e3877fb..33712afb41a 100644 --- a/addons/spreadsheet_dashboard/__manifest__.py +++ b/addons/spreadsheet_dashboard/__manifest__.py @@ -7,9 +7,7 @@ "summary": "Spreadsheet", "description": "Spreadsheet", "depends": ["spreadsheet"], - "demo": [], "installable": True, - "auto_install": False, "license": "LGPL-3", "data": [ "security/security.xml", diff --git a/addons/website_sale_comparison/__manifest__.py b/addons/website_sale_comparison/__manifest__.py index de436fb2546..dfc3b655ce1 100644 --- a/addons/website_sale_comparison/__manifest__.py +++ b/addons/website_sale_comparison/__manifest__.py @@ -10,7 +10,6 @@ To configure product attributes, activate *Attributes & Variants* in the Website Finally, the module comes with an option to display an attribute summary table in product web pages (available in Customize menu). """, - 'author': 'Odoo S.A.', 'category': 'Website/Website', 'version': '1.0', 'depends': ['website_sale'], diff --git a/addons/website_sale_wishlist/__manifest__.py b/addons/website_sale_wishlist/__manifest__.py index f44c75b5e7b..93ef4e45254 100644 --- a/addons/website_sale_wishlist/__manifest__.py +++ b/addons/website_sale_wishlist/__manifest__.py @@ -6,7 +6,6 @@ 'description': """ Allow shoppers of your eCommerce store to create personalized collections of products they want to buy and save them for future reference. """, - 'author': 'Odoo S.A.', 'category': 'Website/Website', 'version': '1.0', 'depends': ['website_sale'], diff --git a/odoo/addons/base/tests/test_module.py b/odoo/addons/base/tests/test_module.py index d4d774fdffe..ca57f0aa630 100644 --- a/odoo/addons/base/tests/test_module.py +++ b/odoo/addons/base/tests/test_module.py @@ -46,7 +46,7 @@ class TestModuleManifest(BaseCase): 'demo_xml': [], 'depends': [], 'description': '', - 'external_dependencies': [], + 'external_dependencies': {}, 'icon': '/base/static/description/icon.png', 'init_xml': [], 'installable': True, diff --git a/odoo/addons/test_lint/tests/test_manifests.py b/odoo/addons/test_lint/tests/test_manifests.py index 510b3a764d4..324fc204d77 100644 --- a/odoo/addons/test_lint/tests/test_manifests.py +++ b/odoo/addons/test_lint/tests/test_manifests.py @@ -1,9 +1,14 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. -from odoo.modules import get_modules -from odoo.modules.module import load_manifest, _DEFAULT_MANIFEST -from odoo.tests import BaseCase +import logging +from ast import literal_eval +from odoo.modules import get_modules +from odoo.modules.module import _DEFAULT_MANIFEST, module_manifest, get_module_path, get_module_resource +from odoo.tests import BaseCase +from odoo.tools import file_open + +_logger = logging.getLogger(__name__) MANIFEST_KEYS = { 'name', 'icon', 'addons_path', 'license', # mandatory keys @@ -13,9 +18,88 @@ MANIFEST_KEYS = { class ManifestLinter(BaseCase): - def test_manifests_keys(self): + + def _load_manifest(self, module): + """Do not rely on odoo/modules/module -> load_manifest + as we want to check manifests content, independently of the + values from _DEFAULT_MANIFEST added automatically by load_manifest + """ + mod_path = get_module_path(module, downloaded=True) + manifest_file = module_manifest(mod_path) + + manifest_data = {} + with file_open(manifest_file, mode='r') as f: + manifest_data.update(literal_eval(f.read())) + + return manifest_data + + def test_manifests(self): for module in get_modules(): with self.subTest(module=module): - manifest_keys = load_manifest(module).keys() - unknown_keys = manifest_keys - MANIFEST_KEYS - self.assertEqual(unknown_keys, set(), f"Unknown manifest keys in module {module!r}. Either there are typos or they must be white listed.") + manifest_data = self._load_manifest(module) + self._test_manifest_keys(module, manifest_data) + self._test_manifest_values(module, manifest_data) + + def _test_manifest_keys(self, module, manifest_data): + manifest_keys = manifest_data.keys() + unknown_keys = manifest_keys - MANIFEST_KEYS + self.assertEqual(unknown_keys, set(), f"Unknown manifest keys in module {module!r}. Either there are typos or they must be white listed.") + + def _test_manifest_values(self, module, manifest_data): + verified_keys = [ + 'application', 'auto_install', + 'summary', 'description', 'author', + 'demo', 'data', 'test', + # todo installable ? + ] + + for key in manifest_data: + value = manifest_data[key] + if key in _DEFAULT_MANIFEST: + if key in verified_keys: + self.assertNotEqual( + value, + _DEFAULT_MANIFEST[key], + f"Setting manifest key {key} to the default manifest value for module {module!r}. " + "You can remove this key from the dict to reduce noise/inconsistencies between manifests specifications" + " and ease understanding of manifest content." + ) + + expected_type = type(_DEFAULT_MANIFEST[key]) + if not isinstance(value, expected_type): + if key != 'auto_install': + _logger.warning( + "Wrong type for manifest value %s in module %s, expected %s", + key, module, expected_type) + elif not isinstance(value, list): + _logger.warning( + "Wrong type for manifest value %s in module %s, expected bool or list", + key, module) + elif key == 'icon': + self._test_manifest_icon_value(module, value) + + def _test_manifest_icon_value(self, module, value): + self.assertTrue( + isinstance(value, str), + f"Wrong type for manifest value icon in module {module!r}, expected string", + ) + self.assertNotEqual( + value, + f"/{module}/static/description/icon.png", + f"Setting manifest key icon to the default manifest value for module {module!r}. " + "You can remove this key from the dict to reduce noise/inconsistencies between manifests specifications" + " and ease understanding of manifest content." + ) + if not value: + _logger.warning( + "Empty value specified as icon in manifest of module %r." + " Please specify a correct value or remove this key from the manifest.", + module) + else: + path_parts = value.split('/') + path = get_module_resource(path_parts[1], *path_parts[2:]) + if not path: + _logger.warning( + "Icon value specified in manifest of module %s wasn't found in given path." + " Please specify a correct value or remove this key from the manifest.", + module) diff --git a/odoo/modules/module.py b/odoo/modules/module.py index 29aa5f8af1f..5ff3a4ce3d3 100644 --- a/odoo/modules/module.py +++ b/odoo/modules/module.py @@ -35,7 +35,7 @@ _DEFAULT_MANIFEST = { 'demo_xml': [], 'depends': [], 'description': '', - 'external_dependencies': [], + 'external_dependencies': {}, #icon: f'/{module}/static/description/icon.png', # automatic 'init_xml': [], 'installable': True,