[18.0][FIX] web_pivot_computed_measure: fix fake fields in tests #3038

kobros-tech · 2024-12-24T21:19:30Z

This fix is the cause for not merging a new migration for web_responsive module to version 18.0.
The migration PR is #3037

OCA-git-bot · 2024-12-24T21:19:34Z

Hi @CarlosRoca13,
some modules you are maintaining are being modified, check this out!

kobros-tech · 2024-12-25T07:29:18Z

I am finding this error all the way:

2024-12-25 07:26:39,542 54005 INFO web_responsive_9 odoo.addons.web_pivot_computed_measure.tests.test_ui_pivot.TestUIPivot.test_ui: waiting for threads: []
2024-12-25 07:26:39,544 54005 INFO web_responsive_9 odoo.addons.web_pivot_computed_measure.tests.test_ui_pivot: ======================================================================
2024-12-25 07:26:39,544 54005 ERROR web_responsive_9 odoo.addons.web_pivot_computed_measure.tests.test_ui_pivot: FAIL: TestUIPivot.test_ui
Traceback (most recent call last):
File "/home/kobros/Workspace/odoo18/web/web_pivot_computed_measure/tests/test_ui_pivot.py", line 62, in test_ui
self.start_tour(
File "/home/kobros/Workspace/odoo18/odoo/odoo/tests/common.py", line 2076, in start_tour
return self.browser_js(url_path=url_path, code=code, ready=ready, timeout=timeout, success_signal="tour succeeded", **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/kobros/Workspace/odoo18/odoo/odoo/tests/common.py", line 2052, in browser_js
self.fail('%s\n\n%s' % (message, error))
AssertionError: The test code "odoo.startTour('web_pivot_computed_measure_tour', {"stepDelay": 100, "keepWatchBrowser": false, "debug": false, "startUrl": "/odoo", "delayToCheckUndeterminisms": 0})" failed

This external module was important for the test:
websocket-client

with it being installed test failed, without it test didn't run!

chienandalu · 2024-12-30T14:50:06Z

web_pivot_computed_measure/static/src/pivot/pivot_model.esm.js

-                if (fieldDef && fieldDef.__computed_id) {
-                    const cm = this._computed_measures.find((item) => {
-                        return item.id === fieldDef.__computed_id;
-                    });
-                    toAnalyze.push(cm.field1, cm.field2);
-                    const toEnableFields = [];
-                    if (!this.metaData.fields[cm.field1].__computed_id) {
-                        toEnableFields.push(cm.field1);
-                    }
-                    if (!this.metaData.fields[cm.field2].__computed_id) {
-                        toEnableFields.push(cm.field2);
+                if (fieldDef) {
+                    if (fieldDef.__computed_id) {


Isn't it the same? You can use the shorter form. Anyway this change isn't needed IMO

if (fieldDef?.__computed_id)

chienandalu · 2024-12-30T14:50:46Z

web_pivot_computed_measure/static/src/pivot/pivot_model.esm.js

@@ -275,17 +288,27 @@ patch(PivotModel.prototype, {
        const fieldNames = Object.keys(this.metaData.fields);
        for (const fieldName of fieldNames) {
            const field = this.metaData.fields[fieldName];
-            if (field.__computed_id) {


Minimize the diff

Suggested change

if (field.__computed_id) {

if (field?.__computed_id) {

chienandalu · 2024-12-30T14:51:44Z

web_pivot_computed_measure/static/src/pivot/pivot_model.esm.js

+                } else {
+                    console.warn(
+                        `Field "${fieldName}" is missing __computed_id`,
+                        field
                    );
                }
+            } else {
+                console.error(`Field "${fieldName}" is undefined in metaData.fields`);


Are these warnings and errors of any use? If this is a corner case where the code is executed but those variables aren't ready, probably the best thing to do is just to ignore it...

I think the test is not aligned with the code, there maybe some promises not functioning.

I tried but I failed as I am not the maintainer, I need help to improve the test case.

@CarlosRoca13 is on holidays these days. Maybe when he comes back could give you some hints 😉

@chienandalu yes, thanks to you too much for being interested.
Happy New Year

chienandalu · 2024-12-30T14:52:05Z

web_pivot_computed_measure/tests/res_users_fake.py

-# Copyright 2022 Tecnativa - Carlos Roca
-# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html)
-
-


sorry sorry, I don't mean to ignore the author :)

chienandalu · 2024-12-30T14:55:21Z

web_pivot_computed_measure/tests/test_ui_pivot.py

+
+        # res_users_model_id = (
+        #     cls.env["ir.model"].search([("model", "=", "res.users")]).id
+        # )
+        # cls.env["ir.model.fields"].create(
+        #     {
+        #         "name": "x_user_year_born",
+        #         "model": "res.users",
+        #         "field_description": "Year Born",
+        #         "ttype": "integer",
+        #         "store": True,
+        #         "model_id": res_users_model_id,
+        #     }
+        # )
+        # cls.env["ir.model.fields"].create(
+        #     {
+        #         "name": "x_user_year_now",
+        #         "model": "res.users",
+        #         "field_description": "Year Now",
+        #         "ttype": "integer",
+        #         "store": True,
+        #         "model_id": res_users_model_id,
+        #     }
+        # )
+


Just remove it?

Suggested change

# res_users_model_id = (

# cls.env["ir.model"].search([("model", "=", "res.users")]).id

# )

# cls.env["ir.model.fields"].create(

# {

# "name": "x_user_year_born",

# "model": "res.users",

# "field_description": "Year Born",

# "ttype": "integer",

# "store": True,

# "model_id": res_users_model_id,

# }

# )

# cls.env["ir.model.fields"].create(

# {

# "name": "x_user_year_now",

# "model": "res.users",

# "field_description": "Year Now",

# "ttype": "integer",

# "store": True,

# "model_id": res_users_model_id,

# }

# )

@chienandalu

I am leaving my comment, as the original code is using a model and we get asked about the fields for this model.

We must create fake fields as while migrating another module errors came from the tests here.

user_year_now, and user_year_born fields

Check this PR please: #3037

yeah, kinda makes sense without having an in depth look

kobros-tech · 2024-12-30T15:29:35Z

@chienandalu see here:

at async TourHelpers.click (http://127.0.0.1:8069/web/assets/1a839a2/web.assets_web.min.js:10674:72)

2024-12-30 15:24:55,997 250 ERROR odoo odoo.addons.web_pivot_computed_measure.tests.test_ui_pivot: FAIL: TestUIPivot.test_ui
Traceback (most recent call last):
File "/__w/web/web/web_pivot_computed_measure/tests/test_ui_pivot.py", line 63, in test_ui
self.start_tour(
File "/opt/odoo/odoo/tests/common.py", line 2076, in start_tour
return self.browser_js(url_path=url_path, code=code, ready=ready, timeout=timeout, success_signal="tour succeeded", **kwargs)
File "/opt/odoo/odoo/tests/common.py", line 2052, in browser_js
self.fail('%s\n\n%s' % (message, error))
AssertionError: The test code "odoo.startTour('web_pivot_computed_measure_tour', {"stepDelay": 100, "keepWatchBrowser": false, "debug": false, "startUrl": "/odoo", "delayToCheckUndeterminisms": 0})" failed

UncaughtTypeError: Cannot read properties of undefined (reading '__computed_id')

test fails, I can remove my commented code but I wanted to show you both errors
1 creating a model in db instead of creating fake fields
2 tour test failure

kobros-tech mentioned this pull request Dec 24, 2024

[18.0][MIG] web_responsive: Migration to 18.0 #3037

Open

kobros-tech force-pushed the 18.0-fix-web_pivot_computed_measure-1 branch 5 times, most recently from 2c27331 to 509ef61 Compare December 24, 2024 22:23

kobros-tech mentioned this pull request Dec 25, 2024

[18.0][BUG] web_pivot_computed_measure: unit tests fail and cause another module migration to fail #3039

Open

kobros-tech force-pushed the 18.0-fix-web_pivot_computed_measure-1 branch from 509ef61 to 1c85a7b Compare December 25, 2024 08:03

chienandalu suggested changes Dec 30, 2024

View reviewed changes

[18.0][FIX] web_pivot_computed_measure: fix fake fields in tests

6512a83

kobros-tech force-pushed the 18.0-fix-web_pivot_computed_measure-1 branch from 1c85a7b to 6512a83 Compare December 30, 2024 15:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[18.0][FIX] web_pivot_computed_measure: fix fake fields in tests #3038

[18.0][FIX] web_pivot_computed_measure: fix fake fields in tests #3038

kobros-tech commented Dec 24, 2024

OCA-git-bot commented Dec 24, 2024

kobros-tech commented Dec 25, 2024

chienandalu Dec 30, 2024

kobros-tech Dec 30, 2024

chienandalu Dec 30, 2024

chienandalu Dec 30, 2024

kobros-tech Dec 30, 2024

chienandalu Dec 30, 2024

kobros-tech Dec 30, 2024

chienandalu Dec 30, 2024

kobros-tech Dec 30, 2024

chienandalu Dec 30, 2024

kobros-tech Dec 30, 2024

chienandalu Dec 30, 2024

kobros-tech commented Dec 30, 2024

		# Copyright 2022 Tecnativa - Carlos Roca
		# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html)

[18.0][FIX] web_pivot_computed_measure: fix fake fields in tests #3038

Are you sure you want to change the base?

[18.0][FIX] web_pivot_computed_measure: fix fake fields in tests #3038

Conversation

kobros-tech commented Dec 24, 2024

OCA-git-bot commented Dec 24, 2024

kobros-tech commented Dec 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kobros-tech commented Dec 30, 2024