New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: ExtensionBuildSystem: Update tests to illustrate #4247 failure #605

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@jcfr
Copy link
Member

jcfr commented Oct 12, 2016

Issue report http://na-mic.org/Mantis/view.php?id=4247

$ ctest -C Release -R py_cmake_slicer_extensions
Test project /home/jcfr/Projects/Slicer-Release/Slicer-build
    Start  7: py_cmake_slicer_extensions_index_build_without_upload
1/4 Test  #7: py_cmake_slicer_extensions_index_build_without_upload ...............   Passed    7.05 sec
    Start  8: py_cmake_slicer_extensions_index_build_with_upload
2/4 Test  #8: py_cmake_slicer_extensions_index_build_with_upload ..................***Failed   12.53 sec
    Start  9: py_cmake_slicer_extensions_index_build_with_upload_using_ctest
3/4 Test  #9: py_cmake_slicer_extensions_index_build_with_upload_using_ctest ......   Passed   33.31 sec
    Start 10: py_cmake_slicer_extensions_index_build_without_upload_using_ctest
4/4 Test #10: py_cmake_slicer_extensions_index_build_without_upload_using_ctest ...   Passed    7.29 sec

To try to make sense of these test results, let's look at what is happening on the factory.

On windows, here are the steps

  1. Every night "factory-south-win7.bat" [1] ends up invoking

     `ctest.exe -S "C:\D\DashboardScripts\factory-south-win7-vs2013-64bits_slicerextensions_release_nightly.cmake" -C Release`
    
  2. Then factory-south-win7-vs2013-64bits_slicerextensions_release_nightly.cmake is a CTest
    script including [2] the generic driver:

    SlicerExtensionsDashboardDriverScript.cmake

    Note: that the test are also using that same generic driver.

  3. Then, the driver script ends up invoking ctest_start, ctest_configure, ctest_build, .... [3]

  4. That ctest_build calls end up building the "Regular" extension build project that is reported to fail above.

Still need to investigate ... to sort things out


[1] https://github.com/Slicer/DashboardScripts/blob/41593b8ec76c7f0aacaf44f9057431e89ed0fa6b/factory-south-win7.bat#L58

[2] https://github.com/Slicer/DashboardScripts/blob/41593b8ec76c7f0aacaf44f9057431e89ed0fa6b/factory-south-win7-vs2013-64bits_slicerextensions_release_nightly.cmake#L105

[3]

ctest_start(${model} TRACK ${track})

@jcfr

This comment has been minimized.

Copy link
Member

jcfr commented Oct 12, 2016

@lassoan @cpinter @pieper @vovythevov @fedorov @ihnorton

Could someone try this on windows too, to check if results (which still do not make sense .. ) are consistent across platform.

Thanks

@jcfr

This comment has been minimized.

Copy link
Member

jcfr commented Oct 12, 2016

@jcfr jcfr force-pushed the jcfr:4247-do-not-prevent-extension-build-if-dependent-extension-tests-fail branch from ccb0f59 to 2dbff91 Oct 12, 2016

@ihnorton

This comment has been minimized.

Copy link
Member

ihnorton commented Oct 12, 2016

Here's log from all tests and then test failure (Windows, x64, debug):

https://gist.github.com/ihnorton/58c9ed1d0dd87fb326407b51ca1f832a

2/3 pass, the issue is:

8:       Start 3: FailingTest
8:   Could not find executable invalid_test
8:   Looked in the following places:
8:   Unable to find executable: invalid_test
8:   invalid_test
8:   invalid_test.exe
8:   Debug/invalid_test
8:   Debug/invalid_test.exe
8:   Debug/invalid_test
8:   Debug/invalid_test.exe
8:
8:   3: Test command:
8:   3/3 Test #3: FailingTest ....................................***Not Run   0.00 sec

(maybe need a WILL_FAIL annotation?)

@jcfr

This comment has been minimized.

Copy link
Member

jcfr commented Oct 12, 2016

Thanks.

maybe need a WILL_FAIL annotation?)

That test is on purpose failing. But the fact it fails should NOT prevent the extensions from building/packaging. That is basically what we are testing here.

Setting WILL_FAIL will cause the test to pass .. which is not desired here. See http://na-mic.org/Mantis/view.php?id=4247

@jcfr jcfr added the bug label Oct 12, 2016

@ihnorton

This comment has been minimized.

Copy link
Member

ihnorton commented Oct 12, 2016

I'm not really following the cmake/python/cmake all the way down here, but from @lassoan's comment in 4274:

How ctest.exe knows that ctest_test failed (from parsing XML output file)? How it could be prevented to return with error code?

I wonder if using RETURN_VALUE would allow to capture the result and avoid non-zero exit?

jcfr added some commits Oct 12, 2016

BUG: midas_api_upload_extension: Do not report warning if metadata ar…
…e empty

This commit is a follow up of r25434 (BUG: Extension upload: Ensure
missing optional metatdata do not cause failure), it fixes the following
warning:

```
8: CMake Warning (dev) at /home/jcfr/Projects/Slicer/Extensions/CMake/MIDASAPIUploadExtension.cmake:75 (message):
8:   warning: CMake variable EXTENSION_CATEGORY is empty !
8: Call Stack (most recent call first):
8:   /home/jcfr/Projects/Slicer/Extensions/CMake/SlicerExtensionPackageAndUploadTarget.cmake:260 (midas_api_upload_extension)
8:
8: This warning is for project developers.  Use -Wno-dev to suppress it.
```
BUG: ExtensionBuildSystem: Update tests to illustrate #4247 problem.
Note: This commit causes the extension build system tests to fail.
      For sake of clarity, it is NOT squashed with the next commit
      that is fixing the problem.

---

On Ubuntu using 3.4.2

  $ ctest -R py_cmake_slicer_extensions
  Test project /home/jcfr/Projects/Slicer-Release/Slicer-build
      Start  7: py_cmake_slicer_extensions_index_build_without_upload
  1/4 Test  #7: py_cmake_slicer_extensions_index_build_without_upload ...............   Passed    7.05 sec
      Start  8: py_cmake_slicer_extensions_index_build_with_upload
  2/4 Test  #8: py_cmake_slicer_extensions_index_build_with_upload ..................***Failed   12.53 sec
      Start  9: py_cmake_slicer_extensions_index_build_with_upload_using_ctest
  3/4 Test  #9: py_cmake_slicer_extensions_index_build_with_upload_using_ctest ......   Passed   33.31 sec
      Start 10: py_cmake_slicer_extensions_index_build_without_upload_using_ctest
  4/4 Test #10: py_cmake_slicer_extensions_index_build_without_upload_using_ctest ...   Passed    7.29 sec

Tested-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>

---

On Windows:

  C:\dev\slcr\s4-vs13\Slicer-build>ctest -C Debug -R py_cmake_slicer_extensions
  Test project C:/dev/slcr/s4-vs13/Slicer-build
      Start  7: py_cmake_slicer_extensions_index_build_without_upload
  1/4 Test  #7: py_cmake_slicer_extensions_index_build_without_upload ...............   Passed  129.20 sec
      Start  8: py_cmake_slicer_extensions_index_build_with_upload
  2/4 Test  #8: py_cmake_slicer_extensions_index_build_with_upload ..................***Failed  106.77 sec
      Start  9: py_cmake_slicer_extensions_index_build_with_upload_using_ctest
  3/4 Test  #9: py_cmake_slicer_extensions_index_build_with_upload_using_ctest ......***Failed  104.04 sec
      Start 10: py_cmake_slicer_extensions_index_build_without_upload_using_ctest
  4/4 Test #10: py_cmake_slicer_extensions_index_build_without_upload_using_ctest ... Passed 125.68 sec

Tested-by: Isaiah Norton <inorton@bwh.harvard.edu>

---

On MacOSX:

  The following tests passed:
	  py_cmake_slicer_extensions_index_build_without_upload
	  py_cmake_slicer_extensions_index_build_with_upload_using_ctest
	  py_cmake_slicer_extensions_index_build_without_upload_using_ctest

  75% tests passed, 1 tests failed out of 4

  Label Time Summary:
  CMake    = 174.82 sec (4 tests)

  Total Test time (real) = 174.95 sec

  The following tests FAILED:
  8 - py_cmake_slicer_extensions_index_build_with_upload (Failed)

Tested-by: Steve Pieper <pieper@isomics.com>

---
BUG: Extension build system: Ignore extension CTest return code. Fixe…
…s #4247

This commit ensures each "EXTENSION_UPLOAD_COMMAND" responsible to configure,
build, test and package any given extension is completely "sandboxed" by
using a wrapper script.

This is required because simply setting the RETURN_VALUE parameter
to the ctest_test() command is not enough to avoid the script from
exiting with error code.

#
# The following wrapper script is required to workaround issue #4247
# and avoid the all extension build from failing if a test of
# extension fail.
#
# Note that as soon as CMake >= 3.6.7 is released, it should be possible
# to remove the wrapper script and simply specify CAPTURE_CMAKE_ERROR
# ctest_test option.
#
# See https://cmake.org/cmake/help/v3.7/command/ctest_test.html
#

Tested-by: Isaiah Norton <inorton@bwh.harvard.edu>
Tested-by: Johan Andruejol <johan.andruejol@kitware.com>
Tested-by: Nicole Aucoin <nicole@bwh.harvard.edu>
Tested-by: Steve Pieper <pieper@isomics.com>

Reported-by: Csaba Pinter <csaba.pinter@queensu.ca>
Reported-by: Andras Lasso <lasso@cs.queensu.ca>

Thanks: All of the above

@jcfr jcfr force-pushed the jcfr:4247-do-not-prevent-extension-build-if-dependent-extension-tests-fail branch from 2dbff91 to 0d2bde4 Oct 12, 2016

@jcfr

This comment has been minimized.

Copy link
Member

jcfr commented Oct 12, 2016

@pieper @ihnorton @cpinter @lassoan @fedorov Et voila, I think the mystery is solved and the problem fixed.

Would be great if you could give it try on Windows and MacOSX.

I will then get started with the release process after dinner.

Thanks
Jc

@pieper

This comment has been minimized.

Copy link
Member

pieper commented Oct 13, 2016

All passed for me!

@jcfr

This comment has been minimized.

Copy link
Member

jcfr commented Oct 13, 2016

Great Thanks for the update

@jcfr

This comment has been minimized.

Copy link
Member

jcfr commented Oct 13, 2016

Now checking on windows to confirm it is fine there..

@jcfr

This comment has been minimized.

Copy link
Member

jcfr commented Oct 13, 2016

Et voila. It now passes on Windows !

C:\D\N\Slicer-1-build\Slicer-build>C:\D\Support\CMake-3.5.0-rc3\bin\ctest.exe -C Release -R py_cmake_slicer_extensions
Test project C:/D/N/Slicer-1-build/Slicer-build
    Start  7: py_cmake_slicer_extensions_index_build_without_upload
1/4 Test  #7: py_cmake_slicer_extensions_index_build_without_upload ...............   Passed   52.57 sec
    Start  8: py_cmake_slicer_extensions_index_build_with_upload
2/4 Test  #8: py_cmake_slicer_extensions_index_build_with_upload ..................   Passed   94.87 sec
    Start  9: py_cmake_slicer_extensions_index_build_with_upload_using_ctest
3/4 Test  #9: py_cmake_slicer_extensions_index_build_with_upload_using_ctest ......   Passed   92.85 sec
    Start 10: py_cmake_slicer_extensions_index_build_without_upload_using_ctest
4/4 Test #10: py_cmake_slicer_extensions_index_build_without_upload_using_ctest ...   Passed   52.24 sec

100% tests passed, 0 tests failed out of 4

Label Time Summary:
CMake    = 292.53 sec (4 tests)

Total Test time (real) = 359.32 sec
BUG: Extension build system: Fix use of wrapper script on windows. Fi…
…xes #4247

This commit ensures the build configuration is passed to the wrapped
command.

It fixes error like this one:


8:   SetMakeCommand:C:\D\Support\CMake-3.5.0-rc3\bin\cmake.exe --build . --config "$(Configuration)"
8:
8:   SetCTestConfiguration:MakeCommand:C:\D\Support\CMake-3.5.0-rc3\bin\cmake.exe --build . --config "$(Configuration)"
8:
8:   Build project
8:
8:   MakeCommand:C:\D\Support\CMake-3.5.0-rc3\bin\cmake.exe --build . --config "$(Configuration)"
8:   Run command: "C:\D\Support\CMake-3.5.0-rc3\bin\cmake.exe" "--build" "." "--config" "$(Configuration)"
8:
8:   Microsoft (R) Build Engine version 12.0.40629.0
8:   [Microsoft .NET Framework, version 4.0.30319.34209]
8:   Copyright (C) Microsoft Corporation. All rights reserved.
8:   Build started 10/12/2016 10:57:46 PM.
8:
8:   Project "C:\D\N\Slicer-1-build\Slicer-build\Extensions\CMake\Testing\build_with_upload-build\TestExtA-build\ALL_BUILD.vcxproj" on node 1 (default targets).
8:   Project "C:\D\N\Slicer-1-build\Slicer-build\Extensions\CMake\Testing\build_with_upload-build\TestExtA-build\ALL_BUILD.vcxproj" (1) is building "C:\D\N\Slic
er-1-build\Slicer-build\Extensions\CMake\Testing\build_with_upload-build\TestExtA-build\ZERO_CHECK.vcxproj" (2) on node 1 (default targets).
8:
8:
8: C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V120\Microsoft.Cpp.Platform.targets(61,5): error MSB8013: This project doesn't contain the Configuration an
d Platform combination of $(Configuration)|x64. [C:\D\N\Slicer-1-build\Slicer-build\Extensions\CMake\Testing\build_with_upload-build\TestExtA-build\ZERO_CHECK.v
cxproj] [C:\D\N\Slicer-1-build\Slicer-build\Extensions\CMake\Testing\build_with_upload-build\TestExtA.vcxproj]
8:
@jcfr

This comment has been minimized.

Copy link
Member

jcfr commented Oct 13, 2016

Gonna integrate now and move forward with da release. Finally

@jcfr

This comment has been minimized.

Copy link
Member

jcfr commented Oct 13, 2016

Integrated in r25440, r25439 and r25438

Thanks again everyone for helping sort this out. 🎉

@jcfr jcfr closed this Oct 13, 2016

@jcfr jcfr deleted the jcfr:4247-do-not-prevent-extension-build-if-dependent-extension-tests-fail branch Oct 13, 2016

@pieper

This comment has been minimized.

Copy link
Member

pieper commented Oct 13, 2016

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment