Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Sep 23, 2020

No description provided.

@fanquake fanquake added the Tests label Sep 23, 2020
@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 23, 2020

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

Coverage

Coverage Change (pull 20004, feeca2363d4c1c78e6dfb40b8684870fe84b084a) Reference (master, ec9b449)
Lines +0.0035 % 90.7088 %
Functions +0.0034 % 86.2602 %
Branches -0.0028 % 52.0393 %

Updated at: 2020-09-29T09:37:30.142032.

@maflcko maflcko force-pushed the 2009-testSignetParse branch from fa4ace1 to faee74f Compare September 24, 2020 13:18
@maflcko
Copy link
Member Author

maflcko commented Sep 28, 2020

cc @kallewoof / @ajtowns 😅

Copy link
Contributor

@kallewoof kallewoof left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

utACK faee74f6c841eea391c08ebbadb045746c3ef77f

@ajtowns
Copy link
Contributor

ajtowns commented Sep 29, 2020

Not really convinced always explicitly passing in an ArgsManager to CreateChainParams is an improvement. Could make it

std::unique_ptr<const CChainParams> CreateChainParams(const std::string& chain, ArgsManager* pargs = nullptr)
{
   ArgsManager& args = (pargs != nullptr ? *pargs : gArgs);
    ...
}

or similar and avoid touching all the other code?

Seems kind of a shame not to add a dummy tx a well to get to 100% line coverage?

@maflcko maflcko force-pushed the 2009-testSignetParse branch from faee74f to fa29b5a Compare September 29, 2020 08:37
@maflcko
Copy link
Member Author

maflcko commented Sep 29, 2020

avoid touching all the other code

Long term goal is to get rid of the gArgs global in Core (though, not the gui), so I think this patch will be done anyway. Left as is for now.

Seems kind of a shame not to add a dummy tx a well to get to 100% line coverage?

Done 💯

@laanwj
Copy link
Member

laanwj commented Sep 30, 2020

ACK fa29b5a

Long term goal is to get rid of the gArgs global in Core (though, not the gui), so I think this patch will be done anyway. Left as is for now.

I think this is overall an improvement, to pass the arguments in. It's also a straightforward, pretty much obviously correct change I'm okay with doing it here.

@laanwj laanwj merged commit 3487e42 into bitcoin:master Sep 30, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Sep 30, 2020
…e tests

fa29b5a test: Add signet witness commitment section parse tests (MarcoFalke)
fa23308 Remove gArgs global from CreateChainParams to aid testing (MarcoFalke)

Pull request description:

ACKs for top commit:
  laanwj:
    ACK fa29b5a

Tree-SHA512: f956407d690decbfb8178bcb8f101d107389fecc3aa7be515f7b0f5ceac26d798c165100f7ddf08cec569beabcc6514862dda23b667cc4fd0a784316784735c2
@maflcko maflcko deleted the 2009-testSignetParse branch September 30, 2020 16:33
fanquake added a commit to fanquake/bitcoin that referenced this pull request Oct 2, 2020
Introduced in bitcoin#20004 (fa29b5a).

```bash
test/validation_tests.cpp:68:88: warning: braces around scalar initializer [-Wbraced-scalar-init]
    BOOST_CHECK(signet_params->GetConsensus().signet_challenge == std::vector<uint8_t>{{OP_TRUE}});
                                                                                       ^~~~~~~~~
/usr/local/include/boost/test/tools/old/interface.hpp:83:6: note: expanded from macro 'BOOST_CHECK'
    (P), BOOST_TEST_STRINGIZE( P ), CHECK, CHECK_PRED, _ )
     ^
/usr/local/include/boost/test/tools/old/interface.hpp:68:61: note: expanded from macro 'BOOST_TEST_TOOL_IMPL'
        BOOST_JOIN( BOOST_TEST_TOOL_PASS_PRED, frwd_type )( P, ARGS ),          \
                                                            ^
/usr/local/include/boost/test/tools/old/interface.hpp:51:47: note: expanded from macro 'BOOST_TEST_TOOL_PASS_PRED2'
                                              ^
1 warning generated.
```
maflcko pushed a commit that referenced this pull request Oct 2, 2020
…n tests

82b70f1 refactor: fix -Wbraced-scalar-init warning in validation tests (fanquake)

Pull request description:

  Introduced in #20004 (fa29b5a).

  ```
  test/validation_tests.cpp:68:88: warning: braces around scalar initializer [-Wbraced-scalar-init]
      BOOST_CHECK(signet_params->GetConsensus().signet_challenge == std::vector<uint8_t>{{OP_TRUE}});
                                                                                         ^~~~~~~~~
  /usr/local/include/boost/test/tools/old/interface.hpp:83:6: note: expanded from macro 'BOOST_CHECK'
      (P), BOOST_TEST_STRINGIZE( P ), CHECK, CHECK_PRED, _ )
       ^
  /usr/local/include/boost/test/tools/old/interface.hpp:68:61: note: expanded from macro 'BOOST_TEST_TOOL_IMPL'
          BOOST_JOIN( BOOST_TEST_TOOL_PASS_PRED, frwd_type )( P, ARGS ),          \
                                                              ^
  /usr/local/include/boost/test/tools/old/interface.hpp:51:47: note: expanded from macro 'BOOST_TEST_TOOL_PASS_PRED2'
                                                ^
  1 warning generated.
  ```

ACKs for top commit:
  practicalswift:
    ACK 82b70f1
  MarcoFalke:
    ACK 82b70f1

Tree-SHA512: ae14acd52e2a0d370a6ee7321f257190c6a44094eb3fa5a6bd85b6bb2b4002e7784589cb34dcf78545238c33cbea38113061b2a46e092f1119731e70932fa469
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 4, 2020
Introduced in bitcoin#20004 (fa29b5a).

```bash
test/validation_tests.cpp:68:88: warning: braces around scalar initializer [-Wbraced-scalar-init]
    BOOST_CHECK(signet_params->GetConsensus().signet_challenge == std::vector<uint8_t>{{OP_TRUE}});
                                                                                       ^~~~~~~~~
/usr/local/include/boost/test/tools/old/interface.hpp:83:6: note: expanded from macro 'BOOST_CHECK'
    (P), BOOST_TEST_STRINGIZE( P ), CHECK, CHECK_PRED, _ )
     ^
/usr/local/include/boost/test/tools/old/interface.hpp:68:61: note: expanded from macro 'BOOST_TEST_TOOL_IMPL'
        BOOST_JOIN( BOOST_TEST_TOOL_PASS_PRED, frwd_type )( P, ARGS ),          \
                                                            ^
/usr/local/include/boost/test/tools/old/interface.hpp:51:47: note: expanded from macro 'BOOST_TEST_TOOL_PASS_PRED2'
                                              ^
1 warning generated.
```
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants