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

Autofind rpc tests --srcdir #8018

Merged
merged 1 commit into from May 9, 2016

Conversation

Projects
None yet
3 participants
@jonasschnelli
Member

jonasschnelli commented May 6, 2016

No description provided.

@jonasschnelli jonasschnelli added the Tests label May 6, 2016

@MarcoFalke

View changes

Show outdated Hide outdated qa/rpc-tests/test_framework/test_framework.py
@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke May 6, 2016

Member

Concept ACK.

Member

MarcoFalke commented May 6, 2016

Concept ACK.

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 6, 2016

Member

Concept ACK.
But should it be build directory containing bitcoind/bitcoin-cli? After out-of-tree builds, there is no guarantee that the executables will be where the source is.

Member

laanwj commented May 6, 2016

Concept ACK.
But should it be build directory containing bitcoind/bitcoin-cli? After out-of-tree builds, there is no guarantee that the executables will be where the source is.

@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli

jonasschnelli May 6, 2016

Member

I only intended to change the default path for the default build setup. Its just annoying entering --srcdir=src all the time. IMO its more convenient to execute the test script from the root build dir instead from the qa/rpc-tests dir (which the current default path is referring to).

Member

jonasschnelli commented May 6, 2016

I only intended to change the default path for the default build setup. Its just annoying entering --srcdir=src all the time. IMO its more convenient to execute the test script from the root build dir instead from the qa/rpc-tests dir (which the current default path is referring to).

@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 7, 2016

Member

I agree, just thought maybe we can change the help message at the same time. But not necessary here.

Member

laanwj commented May 7, 2016

I agree, just thought maybe we can change the help message at the same time. But not necessary here.

@MarcoFalke

This comment has been minimized.

Show comment
Hide comment
@MarcoFalke

MarcoFalke May 7, 2016

Member

This is indeed a simple and uncontroversial improvement. Using BUILDDIR requires some refactoring, which can be done in a later pull.

@jonasschnelli Mind to fix my above nit?

before nit:

  --srcdir=SRCDIR       Source directory containing bitcoind/bitcoin-cli
                        (default: /home/marco/workspace/bitcoin/qa/rpc-
                        tests/test_framework/../../../src)

after nit:

  --srcdir=SRCDIR       Source directory containing bitcoind/bitcoin-cli
                        (default: /home/marco/workspace/bitcoin/src)
Member

MarcoFalke commented May 7, 2016

This is indeed a simple and uncontroversial improvement. Using BUILDDIR requires some refactoring, which can be done in a later pull.

@jonasschnelli Mind to fix my above nit?

before nit:

  --srcdir=SRCDIR       Source directory containing bitcoind/bitcoin-cli
                        (default: /home/marco/workspace/bitcoin/qa/rpc-
                        tests/test_framework/../../../src)

after nit:

  --srcdir=SRCDIR       Source directory containing bitcoind/bitcoin-cli
                        (default: /home/marco/workspace/bitcoin/src)
@jonasschnelli

This comment has been minimized.

Show comment
Hide comment
@jonasschnelli
Member

jonasschnelli commented May 9, 2016

Fixed @MarcoFalke nit.

@MarcoFalke MarcoFalke merged commit 5ea4508 into bitcoin:master May 9, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

MarcoFalke added a commit that referenced this pull request May 9, 2016

Merge #8018: Autofind rpc tests --srcdir
5ea4508 Autofind rpc tests --srcdir (Jonas Schnelli)

codablock added a commit to codablock/dash that referenced this pull request Sep 16, 2017

Merge #8018: Autofind rpc tests --srcdir
5ea4508 Autofind rpc tests --srcdir (Jonas Schnelli)

codablock added a commit to codablock/dash that referenced this pull request Sep 19, 2017

Merge #8018: Autofind rpc tests --srcdir
5ea4508 Autofind rpc tests --srcdir (Jonas Schnelli)

codablock added a commit to codablock/dash that referenced this pull request Dec 21, 2017

Merge #8018: Autofind rpc tests --srcdir
5ea4508 Autofind rpc tests --srcdir (Jonas Schnelli)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment