feat: consider env_misc.platform as hardware #703

murilx · 2024-12-19T14:58:35Z

Add a new logic to the backend where environment_misc.platform will work as a fallback case for when the environment_compatabile field is null in the database. When environment_misc.plaftorm is null, we'll use build.misc.platform, but the logic of the Hardware filter will be kept the same, meaning that the builds returned will be first the build associated with the tests that have that hardware. This means that even if build.misc.platform is null but the build is associated with a test that have environment_compatible or environment_misc.platform not null, it will also be added when filtering for that hardware

This drastically lowers the number of Unknown hardware but they can still occur if both environment_compatible and environment_misc.platform and build.misc.platform are null.

Closes #692

How to test

Add hardware filters from the Hardware Used card, Hardware Tested card and Filter modal and verify that they are correctly being added by comparing with staging. Verify that the commit graph is also working properly, showing the correct graph and changing the commit correctly
Add a platform filter (these are hardwares that do not appear in staging, e.g. acer-R721T-grunt) from the Hardware Used card, Hardware Tested card and Filter modal and make the same verifications as above, if the status are showing correctly (comparing the values in the Hardware Tested card and the value shown in the pie chart) and if the commit graph is working.
Test in other origins to verify that the behaviour is consistent between them

WilsonNet · 2024-12-19T16:13:42Z

backend/kernelCI_app/views/hardwareDetailsView.py

@@ -281,7 +281,7 @@ def handle_test(self, record, tests):

        if status == "ERROR" or status == "FAIL" or status == "MISS":
            tests["platformsFailing"].add(
-                extract_platform(record["environment_misc"])
+                handle_environment_misc(record["environment_misc"]).get("platform")


I had no idea we were already getting platform faililng in hardware

WilsonNet · 2024-12-19T16:14:55Z

backend/kernelCI_app/helpers/tests.py

@@ -0,0 +1,27 @@
+import json
+from typing import Union, TypedDict, Optional
+from kernelCI_app.helpers.filters import UNKNOWN_STRING


filters is probably not the best place for UNKNOWN_STRING maybe helper.constants

but I know it is not being introduced by this PR

I agree with you. I can change this as well if you think it would be better

lets not worry for this for today, we need this quick

WilsonNet · 2024-12-19T16:15:38Z

backend/kernelCI_app/helpers/tests.py

+    platform: str
+
+
+def handle_environment_misc(misc: Union[str, dict, None]) -> Optional[EnvironmentMisc]:


the return here is not Optional, it is always returning

WilsonNet · 2024-12-19T16:16:13Z

backend/kernelCI_app/helpers/tests.py

+        try:
+            misc = json.loads(misc)
+        except json.JSONDecodeError:
+            return parsed_misc


maybe it is a good idea to add the log_message here, for now it just prints but in the future will be doing something more

And keep returning the default value?

omg that was lack of attention, make it the return value back to optional and make it return None

WilsonNet · 2024-12-19T16:22:31Z

dashboard/src/pages/TreeDetails/Tabs/Tests/TestsTab.tsx

+              environmentCompatible={{
+                ...data.testEnvironmentCompatible,
+                ...data.testEnvironmentMisc,
+              }}


useMemo especially when you have to spreads

WilsonNet

it worked on my tests

MarceloRobert · 2024-12-19T19:08:06Z

backend/kernelCI_app/views/treeCommitsHistory.py

@@ -218,8 +220,13 @@ def _process_rows(self, rows: Dict) -> None:
        sanitized_rows = self.sanitize_rows(rows)

        for row in sanitized_rows:
+            hardware_filter = row["hardware_compatibles"]
+            if hardware_filter is None or hardware_filter == UNKNOWN_STRING:


the sanitized rows are simply the original rows but as an object, the row["hardware_compatibles"] hasn't been sanitized to receive UNKNOWN_STRING yet. In other words, the comparison hardware_filter == UNKNOWN_STRING will never be true unless the original row in the database is literally "Unknown", so I think this comparison is not needed

MarceloRobert · 2024-12-19T19:10:51Z

backend/kernelCI_app/views/treeCommitsHistory.py

@@ -218,8 +220,13 @@ def _process_rows(self, rows: Dict) -> None:
        sanitized_rows = self.sanitize_rows(rows)

        for row in sanitized_rows:
+            hardware_filter = row["hardware_compatibles"]


isn't this variable referring to the current hardware of a row? I don't think hardware_filter is a good name for it

MarceloRobert · 2024-12-19T19:14:41Z

backend/kernelCI_app/views/treeDetailsView.py

                self.hardwareUsed.add(test_environment_compatible)
+            else:
+                hardware_filter = test_environment_misc.get("platform")


ditto about hardware_filter var name

Add a new logic to the backend where `environment_misc.platform` will work as a fallback case for when the `environment_compatabile` field is null in the database. When `environment_misc.plaftorm` is null, we'll use `build.misc.platform`, but the logic of the Hardware filter will be kept the same, meaning that the builds returned will be first the build associated with the tests that have that hardware. This means that even if `build.misc.platform` is null but the build is associated with a test that have `environment_compatible` or `environment_misc.platform` not null, it will also be added when filtering for that hardware Closes #692

MarceloRobert

Worked well on my tests, just a note to split the commits into the feature and the formatter refactor for better reviewing

WilsonNet reviewed Dec 19, 2024

View reviewed changes

murilx force-pushed the feat/use-misc-platform-field branch 2 times, most recently from 76ae7f1 to 38cd3a0 Compare December 19, 2024 16:52

murilx self-assigned this Dec 19, 2024

murilx marked this pull request as ready for review December 19, 2024 16:52

WilsonNet approved these changes Dec 19, 2024

View reviewed changes

MarceloRobert reviewed Dec 19, 2024

View reviewed changes

murilx force-pushed the feat/use-misc-platform-field branch from 38cd3a0 to 457bcce Compare December 19, 2024 20:43

MarceloRobert approved these changes Dec 20, 2024

View reviewed changes

murilx merged commit 883517a into main Dec 20, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: consider env_misc.platform as hardware #703

feat: consider env_misc.platform as hardware #703

murilx commented Dec 19, 2024 •

edited

Loading

WilsonNet Dec 19, 2024

WilsonNet Dec 19, 2024

murilx Dec 19, 2024

WilsonNet Dec 19, 2024

WilsonNet Dec 19, 2024

WilsonNet Dec 19, 2024

murilx Dec 19, 2024

WilsonNet Dec 19, 2024

WilsonNet Dec 19, 2024

WilsonNet left a comment

MarceloRobert Dec 19, 2024

MarceloRobert Dec 19, 2024

MarceloRobert Dec 19, 2024

MarceloRobert left a comment

		platform: str


		def handle_environment_misc(misc: Union[str, dict, None]) -> Optional[EnvironmentMisc]:

feat: consider env_misc.platform as hardware #703

feat: consider env_misc.platform as hardware #703

Conversation

murilx commented Dec 19, 2024 • edited Loading

How to test

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

WilsonNet left a 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

MarceloRobert left a comment

Choose a reason for hiding this comment

murilx commented Dec 19, 2024 •

edited

Loading