Match test not run due to executable not found to jenkins <failure>
ClosedPublic

Authored by kossebau on Apr 27 2019, 11:15 PM.

Diff Detail

Repository
R857 CI System Tooling
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
kossebau requested review of this revision.Apr 27 2019, 11:15 PM
kossebau created this revision.

Ran locally on a ctest xml file containing

<Test Status="notrun">
        <Name>plugin-btbrowser_test</Name>
        <Path>./addons/backtracebrowser/autotests</Path>
        <FullName>./addons/backtracebrowser/autotests/plugin-btbrowser_test</FullName>
        <FullCommandLine></FullCommandLine>
        <Results>
                <NamedMeasurement type="numeric/double" name="Processors">
                        <Value>1</Value>
                </NamedMeasurement>
                <NamedMeasurement type="text/string" name="Completion Status">
                        <Value>Unable to find executable</Value>
                </NamedMeasurement>
                <NamedMeasurement type="text/string" name="Command Line">
                        <Value></Value>
                </NamedMeasurement>
                <Measurement>
                        <Value>Unable to find executable: btbrowser_test</Value>
                </Measurement>
        </Results>
</Test>

and got as result

<testcase classname="projectroot.addons.backtracebrowser.autotests" name="plugin_btbrowser_test">
  <failure><![CDATA[Unable to find executable: btbrowser_test]]></failure>
  <system-out><![CDATA[Unable to find executable: btbrowser_test]]></system-out>
</testcase>

Done in the build dir of kate (with a add_test() manipulated to not find the executable) with ctest -T Test and xsltproc /path/to/ci-tooling/templates/ctesttojunit.xsl Testing/20190427-2217/Test.xml

kossebau updated this revision to Diff 57117.Apr 27 2019, 11:59 PM

also handle not-found in summary calculation

<testsuite name="CTest" tests="8" failures="1" errors="0" skipped="1">

fixed now to

<testsuite name="CTest" tests="8" failures="2" errors="0" skipped="0">
bcooksley accepted this revision.Apr 28 2019, 7:10 AM
bcooksley added a subscriber: bcooksley.

This diff itself looks fine, my only question is around the rationale for this change.

Mind going into the background of how CTest can setup a test but not know how to find the binary?

This revision is now accepted and ready to land.Apr 28 2019, 7:10 AM

Mind going into the background of how CTest can setup a test but not know how to find the binary?

It's a combination of the effort to make tests runable without installation in ECM >= 5.38 [1] and an incomplete add_test() syntax (add_test(footest footest) vs. add_test(NAME footest COMMAND footest).

[1] https://phabricator.kde.org/D7198

Thanks for review.

This diff itself looks fine, my only question is around the rationale for this change.

Mind going into the background of how CTest can setup a test but not know how to find the binary?

As @heikobecker said. Example:

add_executable(btbrowser_test ${BtBrowserSrc})
add_test(plugin-btbrowser_test btbrowser_test) # fail, no exe btbrowser_test in same build dir for find_package(ECM 5.38) and newer

This will build fine, only fail at the point when tests are run and the command is executed.
The reason is in the old signature of add_test where the command argument is taken verbatim for the current build dir. While the newer signature (with NAME & COMMAND) also takes a cmake target for the command argument: "If <command> specifies an executable target (created by add_executable()) it will automatically be replaced by the location of the executable created at build time.". So the test binary as generated into the toplevel bin/ directory will be properly called only with the new signature.

add_executable(btbrowser_test ${BtBrowserSrc})
add_test(NAME plugin-btbrowser_test COMMAND btbrowser_test) # no fail, btbrowser_test is interpreted as target, executable name with path to be called resolved from it

Thanks for the explanations, this definitely looks like something we should be catching then.

This revision was automatically updated to reflect the committed changes.