Re: [edk2-devel] running CI locally

["Laszlo Ersek" <lersek@redhat.com>]

On 08/21/20 23:36, Sean Brogan wrote:
> On 8/21/2020 12:23 AM, Laszlo Ersek wrote:

>>    - ArmVirtPkg, DynamicTablesPkg, CryptoPkg, EmulatorPkg, FatPkg,
>>      NetworkPkg, OvmfPkg, PcAtChipsetPkg, SecurityPkg and ShellPkg
>>      are not covered by host-based unit tests -- I think also
>>      expected:
>>
>>> [...]
>>
>>    - I'm not really sure about the following warnings. They were
>>      emitted for a subset of the above packages. I read the docs at
>>
>> <https://github.com/tianocore/edk2/blob/master/.pytool/Readme.md#host-module-inclusion-test---hostunittestdsccompletecheck>
>>
>>      but I still don't understand :)
>
> Each plugin/test has a readme as well.  Not sure if this helps explain
> the HostUnitTestDscCompleteCheck more.
>
> https://github.com/tianocore/edk2/tree/master/.pytool/Plugin/HostUnitTestDscCompleteCheck

I've carefully re-read the explanation now, and (apparently) I do
understand it now :/

Before, I didn't realize that the expression

  "those related to host based unit tests"

stood for the set of INF files where each INF was
MODULE_TYPE=HOST_APPLICATION or
LIBRARY_CLASS=FoobarLib|HOST_APPLICATION.

> As to why you don't see it skipped for ArmVirtPkg or a few others: I
> think this is a bug.
>
> In Azure CI i see it as test passed.
>
> PROGRESS - --Running ArmVirtPkg: Host Unit Test Dsc Complete Check
> Test NO-TARGET --
> PROGRESS - --->Test Success: Host Unit Test Dsc Complete Check Test
> NO-TARGET
>
> When i look at the code
> https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/HostUnitTestDscCompleteCheck/HostUnitTestDscCompleteCheck.py#L59
>
>
> I see that how those packages *.ci.yaml have it configured is
> different than those that show skipped below.  It avoids the skip
> conditions but since those packages don't have any host unit test
> libraries or modules there is no failure.

"OvmfPkg/OvmfPkg.ci.yaml" (as an example) sets both
"HostUnitTestCompilerPlugin.DscPath" and
"HostUnitTestDscCompleteCheck.DscPath" to the empty string.

"ShellPkg/ShellPkg.ci.yaml" (an an example) sets neither attribute at
all.

Both HostUnitTestCompilerPlugin and HostUnitTestDscCompleteCheck check
for the existence of the DscPath attribute, as first step, so both
plugins warn about, and skip, ShellPkg.

OvmfPkg passes the same, first -- i.e., DscPath attribute existence --
check in both plugins. This suggests OvmfPkg is ruled out by *further*
checks in one plugin, and not ruled out by any further checks in the
other plugin.

The next (2nd) check in both plugins seems to be, whether we can
actually find the DSC file pointed-to by the DscPath attribute.
Furthermore, in HostUnitTestDscCompleteCheck, this check is the last one
(no other checks).

I think that this 2nd check *too* should fail for OvmfPkg in *both*
plugins. Because the DscPath attribute, while it exists, is empty.

I don't see why the "DSC path lookup" would fail in
HostUnitTestCompilerPlugin (and so we'd get a warning about OvmfPkg),
but not in HostUnitTestDscCompleteCheck (and so we'd get no warning
about OvmfPkg).

... Is it possible that the 2nd check actually fires in
HostUnitTestDscCompleteCheck too, but we suppress the warning by
returning 0, and not -1?

Compare the return values after the tc.SetSkipped() calls:

[https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/HostUnitTestCompilerPlugin/HostUnitTestCompilerPlugin.py#L103]

100         if AP is None or AP_Path is None or not os.path.isfile(APDSC):
101             tc.SetSkipped()
102             tc.LogStdError("Package HostBasedUnitTest Dsc not found.")
103             return -1
                       ^^

versus

[https://github.com/tianocore/edk2/blob/master/.pytool/Plugin/HostUnitTestDscCompleteCheck/HostUnitTestDscCompleteCheck.py#L73]

70         if abs_dsc_path is None or wsr_dsc_path == "" or not os.path.isfile(abs_dsc_path):
71             tc.SetSkipped()
72             tc.LogStdError("Package Host Unit Test Dsc not found")
73             return 0
                      ^

I guess I could verify if the branch was taken at all, from the standard
error...

Ah, "Build/TestSuites.xml" seems relevant.

... Yes, I'm fairly sure the zero value after the "return" statement, on
line 73, is a typo. Because this is what I have in
"Build/TestSuites.xml" (excerpt re-wrapped here for readability):

> <testsuites>
>     <testsuite
>      id="14"
>      name="OvmfPkg"
>      package="Edk2CiBuild.Edk2.OvmfPkg"
>      errors="0"
>      tests="12"
>      failures="0"
>      skipped="6">
>
>         <testcase
>          classname="Edk2CiBuild.Edk2.OvmfPkg.HostUnitTestCompiler.X64"
>          name="Compile and Run Host-Based UnitTests for Edk2CiBuild.Edk2.OvmfPkg on arch X64"
>          time="0.0002715587615966797">
>             <skipped type="skipped"/>
>             <system-out/>
>             <system-err>Package HostBasedUnitTest Dsc not found.</system-err>
>         </testcase>
>
>         <testcase
>          classname="Edk2CiBuild.Edk2.OvmfPkg.HostUnitTestDscCompleteCheck"
>          name="Check the Edk2CiBuild.Edk2.OvmfPkg Host Unit Test DSC for a being complete"
>          time="0.00020742416381835938">
>             <skipped type="skipped"/>
>             <system-out/>
>             <system-err>Package Host Unit Test Dsc not found</system-err>
>         </testcase>
>
>     </testsuite>
> </testsuites>

Note that both quoted test cases (both plugins) report having been
skipped.

And from the <system-err> element, we can confirm that both plugins
catch OvmfPkg's DscPath attribute being the empty string. It's just that
"HostUnitTestDscCompleteCheck" returns zero on that branch.

> This should probably be a bugzilla on the
> HostUnitTestDscCompleteCheck.py as the reporting is not correct.  Test
> should show up as skipped.

I've now filed:

  https://bugzilla.tianocore.org/show_bug.cgi?id=2924

For now I've assigned it to you, and set BaseTools as the Package
needing the fix. Please update the metadata as necessary :)

>>      - The below seems to be related to the "spell checking in audit
>>        mode" known issues at
>>        <https://github.com/tianocore/edk2/blob/master/.pytool/Readme.md>.
>>
>>        What does "audit mode" mean?
>
> see readme here.
> https://github.com/tianocore/edk2/tree/master/.pytool/Plugin/SpellCheck#configuration
>
>
> Basically runs the tests and reports all the errors as warnings and
> then reports the test as skipped. It was hopefully to encourage
> package maintainers to fix their spelling issues but not breaking the
> build on day 1.

Oh. In OvmfPkg, the CI YAML file says "Fails right now with over 270
errors" :/

>> * One suggestion (in addition to the others made in this thread):
>>
>> - Please add "BaseToolsBuild" to "BaseTools/.gitignore".
>
> There is a bugzilla for this already.
> https://bugzilla.tianocore.org/show_bug.cgi?id=2581

Thanks!

>> * A question:
>>
>> - On github, there is some logic that restricts the full Core CI
>>   build to packages that may be affected by a patch series. Is this
>>   available locally?
>>
>>   According to the Core CI documentation, the "stuart_ci_build"
>>   command can be limited with "-p" / "-a" / "-t" flags; given a
>>   commit range, how do we calculate the "tightest" flag values?
>
> A couple things here.
> You can always look at the azurepipelines files to get a better
> understanding of the CI process.  Hopefully there is no magic (besides
> being able to read yaml).  You can see here:
> https://github.com/tianocore/edk2/blob/master/.azurepipelines/templates/pr-gate-steps.yml#L35
>
>
> It is actually another stuart tool.
> stuart_pr_eval.
> We have bug to document it.
> https://github.com/tianocore/edk2-pytool-extensions/issues/89
> But the python file is pretty well documented. There are 4 policies
> that can trigger the need for a change to build a package.
> See here
> https://github.com/tianocore/edk2-pytool-extensions/blob/master/edk2toolext/invocables/edk2_pr_eval.py#L117

Cool, so I can run something like

  stuart_pr_eval -c .pytool/CISettings.py -p CANDIDATE-PACKAGE-LIST --pr-target TOPIC-BRANCH \
      [output formatting options]

I'll try that next time!

Thank you!
Laszlo