Disable wallet and address book Qt tests on macOS minimal platform #14011

pull ryanofsky wants to merge 1 commits into bitcoin:master from ryanofsky:pr/fuqtmac changing 2 files +23 −0
  1. ryanofsky commented at 8:00 PM on August 20, 2018: contributor

    macOS minimal platform is frequently broken, and these are currently failing with Qt 5.11.1.

    The tests do pass when run on the full cocoa platform (with test_bitcoin-qt -platform cocoa).

    Stack trace from test crash: https://gist.github.com/ryanofsky/3401fb63c52d13d5585e7fc777361f1e

  2. Disable wallet and address book Qt tests on macOS minimal platform
    macOS Qt minimal platform is frequently broken, and these are currently failing
with Qt 5.11.1.

The tests do pass when run on the full cocoa platform
(with `test_bitcoin-qt -platform cocoa`).
    a3197c5294
  3. ryanofsky cross-referenced this on Aug 20, 2018 from issue Add BitcoinApplication & RPCConsole tests by ryanofsky
  4. ryanofsky force-pushed on Aug 20, 2018
  5. in src/qt/test/addressbooktests.cpp:151 in 391cca5158 outdated 
     146 | +        // framework when it tries to look up unimplmented cocoa functions, and
 147 | +        // fails to handle returned nulls
 148 | +        // (https://bugreports.qt.io/browse/QTBUG-49686).
 149 | +       QWARN("Skipping AddressBookTests on mac build with 'minimal' platform set due to Qt bugs. To run AppTests, invoke "
 150 | +              "with 'test_bitcoin-qt -platform cocoa' on mac, or else use a linux or windows build.");
 151 | +        return;
    Empact commented at 8:57 PM on August 20, 2018:

    whitespace inconsistencies here

    ryanofsky commented at 7:20 AM on October 9, 2018:

    Fixed in f7a533f17992e73ced6b57a9725997e040eeaa8a

  6. DrahtBot commented at 9:13 PM on August 20, 2018: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->No more conflicts as of last run.

  7. fanquake added the label macOS on Aug 20, 2018
  8. fanquake added the label Tests on Aug 20, 2018
  9. in src/qt/test/addressbooktests.cpp:20 in 391cca5158 outdated 
      16 | @@ -17,6 +17,7 @@
  17 |  #include <key_io.h>
  18 |  #include <wallet/wallet.h>
  19 |  
  20 | +#include <QApplication>
    promag commented at 11:54 PM on August 20, 2018:

    QGuiApplication?

    ryanofsky commented at 2:51 PM on September 4, 2018:

    The test uses QApplication, so I think this is the right include.

  10. Sjors commented at 6:59 PM on September 7, 2018: member

    I did make check and src/qt/test/test_bitcoin-qt -platform cocoa, but it's still running WalletTests and AddressBookTests. Maybe I need a different -platform argument?

    The test pass for me on macOS 10.13.6 with QT 5.11.1 via Homebrew, with and without this change.

  11. DrahtBot cross-referenced this on Sep 27, 2018 from issue Multiprocess bitcoin by ryanofsky
  12. ryanofsky force-pushed on Sep 27, 2018
  13. fanquake cross-referenced this on Oct 9, 2018 from issue tests: Dry run bench_bitcoin as part "make check" to allow for quick identification of assertion/sanitizer failures in benchmarking code by practicalswift
  14. fanquake commented at 5:07 AM on October 9, 2018: member

    @ryanofsky Could you address any comments/nits here and rebase if needed. I'd like to get this (or something equivalent) merged.

  15. fanquake cross-referenced this on Oct 9, 2018 from issue mac: make check failure on macOS 10.14 by fanquake
  16. ryanofsky commented at 7:31 AM on October 9, 2018: contributor

    Updated 391cca51582269015f23bf931461160a111cc420 -> f7a533f17992e73ced6b57a9725997e040eeaa8a (pr/fuqtmac.2 -> pr/fuqtmac.3) just fixing whitespace.

    From comments above I guess fanquake could reproduce the test failures, and sjors couldn't, but I think it's best just to skip these tests on the uncommonly used minimal mac platform, rather than try to debug.

  17. fanquake commented at 3:29 AM on October 10, 2018: member

    tACK f7a533f

    make check is now fixed for me.

  18. fanquake added this to the "Blockers" column in a project
  19. in src/qt/test/addressbooktests.cpp:146 in f7a533f179 outdated 
     139 | @@ -139,5 +140,16 @@ void TestAddAddressesToSendBook()
 140 |  
 141 |  void AddressBookTests::addressBookTests()
 142 |  {
 143 | +#ifdef Q_OS_MAC
 144 | +    if (QApplication::platformName() == "minimal") {
 145 | +        // Disable for mac on "minimal" platform to avoid crashes inside the Qt
 146 | +        // framework when it tries to look up unimplmented cocoa functions, and
    practicalswift commented at 8:24 PM on October 16, 2018:

    Should be "unimplemented" :-)

  20. ryanofsky force-pushed on Oct 17, 2018
  21. ryanofsky commented at 5:19 PM on October 17, 2018: contributor

    Updated f7a533f17992e73ced6b57a9725997e040eeaa8a -> a3197c5294df4711bab0ff6dcdd061ceab115c7d (pr/fuqtmac.3 -> pr/fuqtmac.4) with spelling fix.

  22. jonasschnelli commented at 6:12 PM on October 17, 2018: contributor

    utACK a3197c5294df4711bab0ff6dcdd061ceab115c7d

  23. fanquake commented at 1:15 AM on October 18, 2018: member

    re-tACK a3197c5

    Merged on top of https://github.com/bitcoin/bitcoin/commit/816fab9ccae568612d5ed90378b4587256925a1e and checked make check -j6.

  24. IlyasRidhuan commented at 6:24 AM on October 19, 2018: none

    testedAck https://github.com/bitcoin/bitcoin/pull/14011/commits/a3197c5294df4711bab0ff6dcdd061ceab115c7d QWarns correctly outputted to logs

    ********* Start testing of WalletTests *********
Config: Using QtTest library 5.11.2, Qt 5.11.2 (x86_64-little_endian-lp64 shared (dynamic) release build; by Clang 9.1.0 (clang-902.0.39.2) (Apple))
PASS   : WalletTests::initTestCase()
WARNING: WalletTests::walletTests() Skipping WalletTests on mac build with 'minimal' platform set due to Qt bugs. To run AppTests, invoke with 'test_bitcoin-qt -platform cocoa' on mac, or else use a linux or windows build.
   Loc: [qt/test/wallettests.cpp(253)]
PASS   : WalletTests::walletTests()
PASS   : WalletTests::cleanupTestCase()
Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 0ms
********* Finished testing of WalletTests *********
********* Start testing of AddressBookTests *********
Config: Using QtTest library 5.11.2, Qt 5.11.2 (x86_64-little_endian-lp64 shared (dynamic) release build; by Clang 9.1.0 (clang-902.0.39.2) (Apple))
PASS   : AddressBookTests::initTestCase()
WARNING: AddressBookTests::addressBookTests() Skipping AddressBookTests on mac build with 'minimal' platform set due to Qt bugs. To run AppTests, invoke with 'test_bitcoin-qt -platform cocoa' on mac, or else use a linux or windows build.
   Loc: [qt/test/addressbooktests.cpp(150)]
PASS   : AddressBookTests::addressBookTests()
PASS   : AddressBookTests::cleanupTestCase()
Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 0ms
********* Finished testing of AddressBookTests *********
PASS qt/test/test_bitcoin-qt (exit status: 0)
  25. promag commented at 2:21 PM on October 19, 2018: member

    Can't reproduce the error on macOS 10.14.

    src/qt/test/test_bitcoin-qt -platform minimal
********* Start testing of URITests *********
Config: Using QtTest library 5.11.1, Qt 5.11.1 (x86_64-little_endian-lp64 shared (dynamic) release build; by Clang 9.1.0 (clang-902.0.39.2) (Apple))
...
  26. ryanofsky commented at 3:16 PM on October 19, 2018: contributor

    I think this is ready for merge. It is platform specific and has tested acks from fanquake #14011 (comment) #14011 (comment) and IlyasRidhuan #14011 (comment) and an untested ACK from jonasschnelli #14011 (comment).

    Sjors and promag have reported in #14011 (comment) and #14011 (comment) that they actually see tests passing without this change, so the issue does seem be a little unpredictable. But this is a good, safe fix for #14349, and also a prerequisite for #11625, which expands qt test coverage and exposes more problems with the qt mac minimal platform.

  27. promag commented at 3:27 PM on October 19, 2018: member

    utACK a3197c5, code change LGTM.

  28. jamesob commented at 5:59 PM on October 19, 2018: member

    utACK https://github.com/bitcoin/bitcoin/pull/14011/commits/a3197c5294df4711bab0ff6dcdd061ceab115c7d

    Code changes are confined to tests and look good to me.

  29. fanquake cross-referenced this on Oct 20, 2018 from issue Problem upgrading to 0.17.0 on macOS High Sierra 10.13.6 by pierrenoizat
  30. sipa merged this on Oct 20, 2018
  31. sipa closed this on Oct 20, 2018
  32. sipa referenced this in commit e754c6e331 on Oct 20, 2018
  33. fanquake removed this from the "Blockers" column in a project
  34. fanquake cross-referenced this on Oct 20, 2018 from issue qa: Revert "Make qt wallet test compatible with qt4" by MarcoFalke
  35. fanquake cross-referenced this on Oct 24, 2018 from issue [0.17] Backports by MarcoFalke
  36. MarcoFalke referenced this in commit 04303ae19d on Oct 24, 2018
  37. MarcoFalke referenced this in commit 8ae2075749 on Oct 25, 2018
  38. MarcoFalke referenced this in commit 9461f98c53 on Oct 25, 2018
  39. toxeus referenced this in commit 0f122c3fbe on Nov 28, 2018
  40. brentkearney cross-referenced this on Feb 25, 2019 from issue make check failure on x64 with MacOS 10.14.3, qt stable 5.12.1. by brentkearney
  41. Earlz referenced this in commit 70e26e041b on Apr 24, 2019
  42. PastaPastaPasta referenced this in commit b50c6838f2 on Jun 27, 2021
  43. PastaPastaPasta referenced this in commit 994fd5e385 on Jun 28, 2021
  44. PastaPastaPasta referenced this in commit dc36c2339a on Jun 29, 2021
  45. PastaPastaPasta referenced this in commit c335d5c6ce on Jul 1, 2021
  46. PastaPastaPasta referenced this in commit bc29354969 on Jul 1, 2021
  47. PastaPastaPasta referenced this in commit bba8c00a72 on Jul 1, 2021
  48. PastaPastaPasta referenced this in commit dbe9d6a018 on Jul 3, 2021
  49. bitcoin locked this on Sep 8, 2021
Contributors
ryanofskyEmpactDrahtBotpromagSjorsfanquakepracticalswiftjonasschnelliIlyasRidhuanjamesob
Labels
macOSTests
Linked (view graph)
#11625 Add BitcoinApplication & RPCConsole tests#14092 tests: Dry run bench_bitcoin as part "make check" to allow for quick identification of assertion/sanitizer failures in benchmarking code#14328 [0.17] Backports#14349 mac: make check failure on macOS 10.14#14414 Problem upgrading to 0.17.0 on macOS High Sierra 10.13.6#14527 qa: Revert "Make qt wallet test compatible with qt4"#15480 make check failure on x64 with MacOS 10.14.3, qt stable 5.12.1.
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-19 06:54 UTC