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

Fix MacOS tests #3320

Merged
merged 6 commits into from Jun 19, 2018

Conversation

6 participants
@str4d
Copy link
Contributor

str4d commented Jun 5, 2018

Includes code cherry-picked from upstream PR bitcoin/bitcoin#8270.

ChoHag and others added some commits Jun 26, 2016

@str4d

This comment has been minimized.

Copy link
Contributor

str4d commented Jun 5, 2018

@zkbot try

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jun 5, 2018

⌛️ Trying commit 72835bf with merge fdd40d5...

zkbot added a commit that referenced this pull request Jun 5, 2018

Auto merge of #3320 - str4d:macos-tests, r=<try>
Fix MacOS tests

Includes code cherry-picked from upstream PR bitcoin/bitcoin#8270.

@str4d str4d added this to In Progress in Zcashd Team Jun 5, 2018

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jun 5, 2018

💔 Test failed - pr-try

@str4d

This comment has been minimized.

Copy link
Contributor

str4d commented Jun 5, 2018

Transient failure on one of the supported builders.

Looks like most of the errors are fixed. I need to do some more investigation into why sec-hard is still not detecting the binary type. And FWICT the RPC test failures are due to performance / memory limitations of the worker.

str4d added some commits Jun 5, 2018

Add Mach-O 64-bit detection to security-check.py
Fixes sec-hard test on MacOS CI worker. At some point we can extend this with
actual security hardening checks.
Fix cached_witnesses_empty_chain test failure on MacOS
Assertion error format is different, so match only on the assertion.

@str4d str4d force-pushed the str4d:macos-tests branch from 72835bf to 2802e32 Jun 7, 2018

@str4d

This comment has been minimized.

Copy link
Contributor

str4d commented Jun 7, 2018

Simple sec-hard fix: I forgot to index into the tuple that struct.unpack returns.

@zkbot try

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jun 7, 2018

⌛️ Trying commit 2802e32 with merge 0060890...

zkbot added a commit that referenced this pull request Jun 7, 2018

Auto merge of #3320 - str4d:macos-tests, r=<try>
Fix MacOS tests

Includes code cherry-picked from upstream PR bitcoin/bitcoin#8270.
@str4d

This comment has been minimized.

Copy link
Contributor

str4d commented Jun 7, 2018

Uncovered another necessary sec-hard change.

@zkbot try

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jun 7, 2018

⌛️ Trying commit 341a22a with merge 22a64ba...

zkbot added a commit that referenced this pull request Jun 7, 2018

Auto merge of #3320 - str4d:macos-tests, r=<try>
Fix MacOS tests

Includes code cherry-picked from upstream PR bitcoin/bitcoin#8270.
@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jun 7, 2018

☀️ Test successful - pr-try
State: approved= try=True

@str4d

This comment has been minimized.

Copy link
Contributor

str4d commented Jun 8, 2018

Okay, the only CI stage now failing is the RPC tests. It looks like the newer MacOS worker being set up encounters build issues due to using a newer LLVM, so we won't be able to immediately use it to run the RPC tests. So I'm happy to have this reviewed and merged as-is.

@str4d str4d moved this from In Progress to In Review in Zcashd Team Jun 8, 2018

@bitcartel

This comment has been minimized.

Copy link
Contributor

bitcartel commented Jun 8, 2018

@str4d Who should be assigned as reviewer? I skimmed the changes and "it looks fine" but I'm not testing on actual Mac hardware.

@mdr0id mdr0id self-requested a review Jun 14, 2018

@ebfull

This comment has been minimized.

Copy link
Contributor

ebfull commented Jun 14, 2018

Also LGTM but cannot test.

@mdr0id

This comment has been minimized.

Copy link
Contributor

mdr0id commented Jun 15, 2018

@str4d Currently running tests on native MacOSX (Darwin mdevs-MacBook-Air.local 17.2.0 Darwin Kernel Version 17.2.0: Fri Sep 29 18:27:05 PDT 2017; root:xnu-4570.20.62~3/RELEASE_X86_64 x86_64), and build is stable/runs.

Minor issues found:

  • dependency check doesn't validate missing pyblake2, zmq modules, prior to execution

  • the #! change for python2 threw a fit, essentially due to a invalid path:
    resolved via sudo ln -sf /usr/bin/python2.7 /usr/local/bin/python2, otherwise ALL rpc tests fail per
    full_test_suite.py execution

  • some of the code appears to use old versions of modules that use self.nodes where nodes is no longer an attribute of X object.

Below is the RPC log output (rpc-tests.sh):

Tests completed: 48
successes 29; failures: 19

Failing tests: wallet_protectcoinbase.py wallet_mergetoaddress.py wallet_nullifiers.py listtransactions.py mempool_resurrect_test.py txn_doublespend.py txn_doublespend.py --mineblock getchaintips.py mempool_spendcoinbase.py mempool_reorg.py mempool_tx_expiry.py httpbasics.py proxy_test.py nodehandling.py blockchain.py zcjoinsplit.py zcjoinsplitdoublespend.py reorg_limit.py zmq_test.py

Otherwise majority of the tests did successfully run and pass. I am running the test suite again to get a full pass/fail list.

@str4d What python interpreter are you running against?

@mdr0id

This comment has been minimized.

Copy link
Contributor

mdr0id commented Jun 16, 2018

Seeing a few tests hang/fail for http related reasons, but making some progress:

Tests completed: 48
successes 39; failures: 9

Failing tests: wallet_protectcoinbase.py wallet_shieldcoinbase.py wallet_mergetoaddress.py wallet.py wallet_nullifiers.py mempool_nu_activation.py mempool_tx_expiry.py zcjoinsplit.py zkey_import_export.py

@str4d

This comment has been minimized.

Copy link
Contributor

str4d commented Jun 17, 2018

@mdr0id

mdr0id approved these changes Jun 17, 2018

Copy link
Contributor

mdr0id left a comment

tested ACK

@str4d str4d added this to the v1.1.2 milestone Jun 17, 2018

@bitcartel

This comment has been minimized.

Copy link
Contributor

bitcartel commented Jun 18, 2018

@zkbot retry

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jun 18, 2018

⌛️ Trying commit 341a22a with merge 7ace9f9...

zkbot added a commit that referenced this pull request Jun 18, 2018

Auto merge of #3320 - str4d:macos-tests, r=<try>
Fix MacOS tests

Includes code cherry-picked from upstream PR bitcoin/bitcoin#8270.
@bitcartel
Copy link
Contributor

bitcartel left a comment

utACK. I agree with @str4d's comment above as long as the PR passes CI testing.

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jun 19, 2018

☀️ Test successful - pr-try
State: approved= try=True

@bitcartel

This comment has been minimized.

Copy link
Contributor

bitcartel commented Jun 19, 2018

@zkbot r+

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jun 19, 2018

📌 Commit 341a22a has been approved by bitcartel

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jun 19, 2018

⌛️ Testing commit 341a22a with merge 4e3ff06...

zkbot added a commit that referenced this pull request Jun 19, 2018

Auto merge of #3320 - str4d:macos-tests, r=bitcartel
Fix MacOS tests

Includes code cherry-picked from upstream PR bitcoin/bitcoin#8270.

@bitcartel bitcartel moved this from In Review to Released (Merged in Master) in Zcashd Team Jun 19, 2018

@zkbot

This comment has been minimized.

Copy link
Collaborator

zkbot commented Jun 19, 2018

☀️ Test successful - pr-merge
Approved by: bitcartel
Pushing 4e3ff06 to master...

@zkbot zkbot merged commit 341a22a into zcash:master Jun 19, 2018

1 check passed

homu Test successful
Details

@str4d str4d deleted the str4d:macos-tests branch Jul 2, 2018

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