openbmc / openbmc-build-scripts

Apache License 2.0
17 stars 50 forks source link

unit-test.py: Problems with analyzes #38

Open ewalkusx opened 2 weeks ago

ewalkusx commented 2 weeks ago

All BMC downstream repos should generate UT coverage report using upstream script, but there are problems with analyzes. There is no coverage report if the previous analyzes fail.

  1. clang-tidy With clang 17 there are compiler errors for nested tuple construction with flag -std=c++23. This is resolved in clang 18, but we are still waiting for clang update: https://gerrit.openbmc.org/c/openbmc/openbmc-build-scripts/+/69586
  2. sanitizers: linker error with -Db_lundef=false and -Db_sanitize=address,undefined

I can propose 2 solutions: 1) changing the order of the analyzes https://gerrit.openbmc.org/c/openbmc/openbmc-build-scripts/+/72247 2) adding coverage report to 'TEST_ONLY' option

--- a/scripts/unit-test.py
+++ b/scripts/unit-test.py
@@ -921,6 +921,7 @@ class Meson(BuildSystem):
             # is supported by meson's build types. Configure it manually.
             meson_flags.append("-Ddebug=true")
             meson_flags.append("-Doptimization=g")
+            meson_flags.append("-Db_coverage=true")
         else:
             meson_flags.append("--buildtype=debugoptimized")
         if OptionKey("tests") in meson_options:
@@ -1186,6 +1187,8 @@ class Package(object):
         system.test()
         if not TEST_ONLY:
             system.analyze()
+        else:
+            check_call_cmd("ninja", "-C", "build", "coverage-html")

     def test(self):
         for system in self.build_systems():

Any other ideas? 🙂

amboar commented 2 weeks ago

Perhaps add a coverage() method to the BuildSystem class in https://github.com/openbmc/openbmc-build-scripts/blob/master/scripts/unit-test.py#L526 ? That way it can be run independent of other analysis.

ewalkusx commented 2 weeks ago

https://gerrit.openbmc.org/c/openbmc/openbmc-build-scripts/+/72727 Add an optional parameter ANALYSES to enable the ability to run only the listed analyses in the given order.

I think this solution has more potential.