💬 fjahr commented on pull request "test: Add coverage for dumptxoutset failure robustness":
(https://github.com/bitcoin/bitcoin/pull/30817#discussion_r1745035071)
done

💬 fjahr commented on pull request "test: Add coverage for dumptxoutset failure robustness":
(https://github.com/bitcoin/bitcoin/pull/30817#issuecomment-2330925948)
> You can also remove UniValue error

Done in the refactoring commit, thanks!

👍 TheCharlatan approved a pull request: "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange, drop CRPCSignals & g_best_block"
(https://github.com/bitcoin/bitcoin/pull/30409#pullrequestreview-2282242465)
ACK 6c419285720fe5a954e01f09cbe053a0f51bef47

But I think it would be good to get more review here. I am fairly convinced that the changes to when the notifications are issued now are good, but the change in logic is a bit tricky to follow.

💬 fanquake commented on pull request "test: fixing failing system_tests/run_command under some Locales":
(https://github.com/bitcoin/bitcoin/pull/30788#issuecomment-2330951266)
Backported to 28.x in #30762.

💬 maflcko commented on pull request "test: Use std::span and std::string_view for raw data":
(https://github.com/bitcoin/bitcoin/pull/30796#discussion_r1745076473)
Sure, added compile time check that ensures `std::is_integral_v<uint8_t>`.

Also, fixed typo in commit message.

Should be trivial to re-ACK

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2330975711)
The merge conflicts are addressed in #29032 and #29346. I plan to do another rebase round when more of the interface pull requests are merged, e.g. #30409.

👍 TheCharlatan approved a pull request: "test: Use std::span and std::string_view for raw data"
(https://github.com/bitcoin/bitcoin/pull/30796#pullrequestreview-2282318162)
ACK faecca9a85c1c1917d024f55cca34d19cc94f3b9

💬 fjahr commented on pull request "test: Use std::span and std::string_view for raw data":
(https://github.com/bitcoin/bitcoin/pull/30796#issuecomment-2331002451)
re-ACK https://github.com/bitcoin/bitcoin/commit/faecca9a85c1c1917d024f55cca34d19cc94f3b9

Only changes since last review were usage of `std::to_integer` and some shuffling of newline characters.

👍 TheCharlatan approved a pull request: "depends: build libevent with `-D_GNU_SOURCE`"
(https://github.com/bitcoin/bitcoin/pull/30743#pullrequestreview-2282333673)
ACK 556775408797d8e27154c3edaf139820b0979cce

The reproducibility issues seems to be unrelated to this change, so I don't think this should be blocked by it.

💬 somethingbeta commented on issue "Test Framework - test_framework.test_node.FailedToStartError: No RPC credentials":
(https://github.com/bitcoin/bitcoin/issues/30818#issuecomment-2331029828)
> > Error: Specified data directory "/tmp/bitcoin_func_test_u9zpt_2g/node0" does not exist.
>
> This seems to be the first error, so it would be good to debug this first. Can you check whether `/usr/src/bitcoin/test/cache/node0` exists and whether the following is successful:
>
> > 2024-09-04T18:07:10.780000Z TestFramework (DEBUG): Copy cache directory /usr/src/bitcoin/test/cache/node0 to node 0
>
> Given that you claim that `bitcoin.conf` exists (presumably in this dir), the error mess
...

💬 TheCharlatan commented on pull request "contrib: fix check-deps.sh to check for weak symbols":
(https://github.com/bitcoin/bitcoin/pull/30415#discussion_r1745115109)
I think we are trying to consistently use `cmake --build` over `make `.

👍 TheCharlatan approved a pull request: "contrib: fix check-deps.sh to check for weak symbols"
(https://github.com/bitcoin/bitcoin/pull/30415#pullrequestreview-2282355222)
ACK 3e4312eef78f233eb7ae1d7d85e497de34144f2e

💬 somethingbeta commented on issue "Test Framework - test_framework.test_node.FailedToStartError: No RPC credentials":
(https://github.com/bitcoin/bitcoin/issues/30818#issuecomment-2331040665)
> Running the Windows tests on Wine in Linux is obviously better than nothing, but it would be better to run them directly on Windows. However, I am not sure what the easiest way to do this would be. For now, editing the config ini to adjust the paths is required.

When you say edit the config.ini paths, what would i be editing here please?

💬 fanquake commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1745128233)
> Does export LC_ALL=C.UTF-8 make any difference?

Yes, this seems to fix the failures when running directly under wine. Can followup with an issues/docs.

💬 maflcko commented on issue "Test Framework - test_framework.test_node.FailedToStartError: No RPC credentials":
(https://github.com/bitcoin/bitcoin/issues/30818#issuecomment-2331047225)
For WSL, can you share exact steps to reproduce, please? I guess they are https://github.com/bitcoin/bitcoin/blob/28.x/doc/build-windows.md ?



> > Running the Windows tests on Wine in Linux is obviously better than nothing, but it would be better to run them directly on Windows. However, I am not sure what the easiest way to do this would be. For now, editing the config ini to adjust the paths is required.
>
> When you say edit the config.ini paths, what would i be editing here please?
...

💬 maflcko commented on pull request "build: Introduce CMake-based build system":
(https://github.com/bitcoin/bitcoin/pull/30454#discussion_r1745136573)
Longer term, I wonder if it makes sense for the build system (and guix) to produce a "test kit" that contains the test config ini file required to run the tests (unit and functional ones) as well as other required stuff, so that on native Windows, one can simply move the exe files to a folder inside the "test kit" and then run the kit to execute all tests.

🚀 fanquake merged a pull request: "depends: build libevent with `-D_GNU_SOURCE`"
(https://github.com/bitcoin/bitcoin/pull/30743)

💬 fanquake commented on pull request "depends: build libevent with `-D_GNU_SOURCE`":
(https://github.com/bitcoin/bitcoin/pull/30743#issuecomment-2331063559)
Backported to 28.x in #30762.

👋 fanquake's pull request is ready for review: "[28.x] rc backports"
(https://github.com/bitcoin/bitcoin/pull/30762)

💬 somethingbeta commented on issue "Test Framework - test_framework.test_node.FailedToStartError: No RPC credentials":
(https://github.com/bitcoin/bitcoin/issues/30818#issuecomment-2331066512)
> For WSL, can you share exact steps to reproduce, please? I guess they are https://github.com/bitcoin/bitcoin/blob/28.x/doc/build-windows.md ?
>
> > > Running the Windows tests on Wine in Linux is obviously better than nothing, but it would be better to run them directly on Windows. However, I am not sure what the easiest way to do this would be. For now, editing the config ini to adjust the paths is required.
> >
> >
> > When you say edit the config.ini paths, what would i be editing h
...

💬 maflcko commented on issue "Test Framework - test_framework.test_node.FailedToStartError: No RPC credentials":
(https://github.com/bitcoin/bitcoin/issues/30818#issuecomment-2331088089)
Ok, thanks for confirming.

I don't have WSL, so I can't test this. However running the functional tests inside WSL was never supported, IIRC.

Even if you work around your initial issue, the test framework and individual tests have to modified for the Wine environment.

So my recommendation would be to run the tests on Windows directly. However that isn't possible out-of-the-box either and requires modifying the config ini file.