chore: grep for failures in CI #6339

mvadari · 2026-02-06T19:23:16Z

High Level Overview of Change

This PR adjusts the CI tests to make it easier to spot errors without needing to sift through the thousands of lines of output.

Context of Change

Easier to sort through the test output, especially on mobile where there is no ctrl-f

Type of Change

Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)

API Impact

N/A

Test Plan

https://github.com/XRPLF/rippled/actions/runs/21763273024/job/62792265122?pr=6339
See "Show test failure summary" section

Copilot

Pull request overview

This PR aims to make CI failures easier to spot by capturing embedded unit test output into a log file and printing a small “failure” summary, alongside a small wording tweak to a parse-error log line in the JTx test environment.

Changes:

Update a JTx parse error log line from “parse failed” to “failure to parse”.
Pipe embedded unit test output through tee into unittest.log while preserving the original test exit code.
Add a post-failure workflow step to grep the unit test log for failure indicators.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
`src/test/jtx/impl/Env.cpp`	Adjusts parse failure log phrasing in `Env::autofill`.
`.github/workflows/reusable-build-test-config.yml`	Captures embedded unittest output to `unittest.log` and adds a grep-based failure summary step.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

.github/workflows/reusable-build-test-config.yml

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-06T20:15:48Z

src/test/app/Vault_test.cpp

                tx[sfAssetsMaximum] = "18446744073709551617e5";  // uint64 max + 1
                env(tx, THISLINE);
-                BEAST_EXPECT(false);
+                fail("Expected parse_error for mantissa larger than uint64 max");


fail("Expected parse_error...") doesn’t include file/line context in the failure reason (unlike BEAST_EXPECT / BEAST_EXPECTS). Since this is in a negative-test path meant to aid debugging, consider using BEAST_EXPECTS(false, ...) (or the fail(reason, __FILE__, __LINE__) overload) so the output includes the source location when the parse_error is not thrown.

Suggested change

fail("Expected parse_error for mantissa larger than uint64 max");

BEAST_EXPECTS(false, "Expected parse_error for mantissa larger than uint64 max");

codecov · 2026-02-06T20:31:32Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.9%. Comparing base (e11f619) to head (d4bc64b).

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #6339     +/-   ##
=========================================
- Coverage     79.9%   79.9%   -0.0%     
=========================================
  Files          840     840             
  Lines        65514   65514             
  Branches      7245    7246      +1     
=========================================
- Hits         52359   52355      -4     
- Misses       13155   13159      +4

see 5 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

bthomee · 2026-02-09T19:12:48Z

.github/workflows/reusable-build-test-config.yml

+          ./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" 2>&1 | tee unittest.log
+          exit ${PIPESTATUS[0]}


My understanding was that an exit command will prevent the rest of the commands to run, even if the exit code is 0.

The reason it works here is because there are no non-failure() steps that follow this, but if we ever need to add another step then this might lead to unexpected behavior (until the developer notices the exit statement which, since it's unusual to see them outside of if-statements, is easy to overlook).

Would the following work?

Suggested change

./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" 2>&1 | tee unittest.log

exit ${PIPESTATUS[0]}

./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" 2>&1 | tee unittest.log || exit ${PIPESTATUS[0]}

Augment was concerned that PIPESTATUS might be overwritten in this case, so I changed it to something related.

.github/workflows/reusable-build-test-config.yml

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-12T22:11:22Z

include/xrpl/beast/unit_test/suite.h

 #include <boost/throw_exception.hpp>

 #include <ostream>
+#include <source_location>


Introducing std::source_location requires C++20 support. If this repository/toolchain is still on C++17, this will fail to compile. Consider using a portable alternative (e.g., boost::source_location / existing project location macros) or gate this behind a C++20 feature check, or keep the old signature.

Suggested change

#include <source_location>

#if defined(__has_include)

# if __has_include(<source_location>)

# include <source_location>

# define BEAST_HAS_STD_SOURCE_LOCATION 1

# endif

#endif

Copilot · 2026-02-12T22:11:23Z

include/xrpl/beast/unit_test/suite.h

    template <class = void>
    void
-    fail(std::string const& reason = "");
+    fail(std::string const& reason = "", std::source_location const& loc = std::source_location::current());


Introducing std::source_location requires C++20 support. If this repository/toolchain is still on C++17, this will fail to compile. Consider using a portable alternative (e.g., boost::source_location / existing project location macros) or gate this behind a C++20 feature check, or keep the old signature.

Copilot · 2026-02-12T22:11:23Z

include/xrpl/beast/unit_test/suite.h

+suite::fail(std::string const& reason, std::source_location const& loc)
 {
    propagate_abort();
    runner_->fail(reason);


The new loc parameter is unused, which makes the API change hard to justify and can trigger warnings under stricter builds. Either incorporate loc into the failure reporting (e.g., pass it to runner_->fail(...) or append file/line to reason) or remove the parameter until it’s wired through.

Suggested change

runner_->fail(reason);

runner_->fail(detail::make_reason(reason, loc.file_name(), static_cast<int>(loc.line())));

Copilot · 2026-02-12T22:11:23Z

src/test/app/Vault_test.cpp

+            env.set_parse_failure_expected(true);
            try
            {
                tx[sfAssetsMaximum] = "18446744073709551617e5";  // uint64 max + 1
                env(tx, THISLINE);
-                BEAST_EXPECT(false);
+                BEAST_EXPECT(false, "Expected parse_error for mantissa larger than uint64 max");
            }
            catch (parse_error const& e)
            {
                using namespace std::string_literals;
-                BEAST_EXPECT(
-                    e.what() ==
-                    "invalidParamsField 'tx_json.AssetsMaximum' has invalid "
-                    "data."s);
+                BEAST_EXPECT(e.what() == "invalidParamsField 'tx_json.AssetsMaximum' has invalid data."s);
            }
+            env.set_parse_failure_expected(false);


set_parse_failure_expected(false) won’t run if an unexpected exception type is thrown (or an early abort/throw occurs), which can leak the 'expected parse failure' state into subsequent checks. Prefer a scope guard/RAII helper that resets the flag in its destructor, or structure this so the reset happens on all exit paths.

Copilot · 2026-02-12T22:11:23Z

.github/workflows/reusable-build-test-config.yml

+          set -o pipefail
+          ./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" 2>&1 | tee unittest.log
+
+      - name: Show test failure summary


These commands assume a bash + GNU userland (set -o pipefail, tee, grep). On Windows runners (and sometimes macOS depending on shell), the default shell may be PowerShell and grep may be unavailable, causing the workflow to fail or skip the intended behavior. Specify shell: bash explicitly for these steps and/or add OS-conditional implementations (PowerShell equivalent on Windows).

This reverts commit 78f11bc.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-12T22:21:30Z

src/test/app/AccountDelete_test.cpp

        // Burn a chunk of alice's funds so she only has 1 XRP remaining in
        // her account.
-        env(noop(alice), fee(env.balance(alice) - XRP(1)));
+        env(noop(alice), fee(env.balance(alice) - XRP(1)), ter(telINSUF_FEE_P));


This change makes the transaction expect to fail with telINSUF_FEE_P, but the comment on lines 565-566 states "Burn a chunk of alice's funds so she only has 1 XRP remaining". If the transaction is expected to fail, alice's funds would not be burned, and she would still have the full reserve (200 XRP based on line 562). This would break the test logic since line 571 expects acctDelFee > env.balance(alice), which wouldn't be true if alice still has 200 XRP. The original code (without ter(telINSUF_FEE_P)) was correct - the noop transaction should succeed and burn the funds.

Suggested change

env(noop(alice), fee(env.balance(alice) - XRP(1)), ter(telINSUF_FEE_P));

env(noop(alice), fee(env.balance(alice) - XRP(1)));

Copilot · 2026-02-12T22:21:31Z

src/test/app/AccountDelete_test.cpp

        // alice again attempts to delete her account.  This time she specifies
        // her current balance in XRP.  Again the transaction fails.
-        BEAST_EXPECT(env.balance(alice) == XRP(1));
+        BEAST_EXPECT(env.balance(alice) == XRP(1000));


This assertion now expects alice to have 1000 XRP, but based on the comment on lines 565-566, alice should have only 1 XRP remaining after burning most of her funds. The test logic depends on alice having a small balance (1 XRP) to verify that account deletion fails when the balance is insufficient to pay the fee. Changing this to XRP(1000) breaks the test's intended behavior and would cause the assertion to fail.

Suggested change

BEAST_EXPECT(env.balance(alice) == XRP(1000));

BEAST_EXPECT(env.balance(alice) == XRP(1));

mvadari added 2 commits February 6, 2026 14:21

grep for failures

2d529a2

add test failure

22a25cf

mvadari added the DraftRunCI Normally CI does not run on draft PRs. This opts in. label Feb 6, 2026

mvadari added 3 commits February 6, 2026 14:31

another failure test

3fddd79

simplify output

82d7645

roll back test failures

78f11bc

mvadari added Trivial Simple change with minimal effect, or already tested. Only needs one approval. and removed DraftRunCI Normally CI does not run on draft PRs. This opts in. labels Feb 6, 2026

mvadari marked this pull request as ready for review February 6, 2026 19:52

Copilot AI review requested due to automatic review settings February 6, 2026 19:52

Copilot started reviewing on behalf of mvadari February 6, 2026 19:52 View session

mvadari changed the title ~~grep for failures~~ chore: grep for failures in CI Feb 6, 2026

Copilot AI reviewed Feb 6, 2026

View reviewed changes

.github/workflows/reusable-build-test-config.yml Show resolved Hide resolved

mvadari added 2 commits February 6, 2026 15:11

fix parse failure for real

1294aac

change string

7e804a5

Copilot AI review requested due to automatic review settings February 6, 2026 20:11

Copilot started reviewing on behalf of mvadari February 6, 2026 20:12 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

mvadari requested a review from bthomee February 9, 2026 18:21

bthomee reviewed Feb 9, 2026

View reviewed changes

mvadari added 3 commits February 10, 2026 17:24

Merge remote-tracking branch 'upstream/develop' into better-erroring

d4bc64b

better exit code

4d385e9

Merge branch 'develop' into better-erroring

8760f80

Copilot AI review requested due to automatic review settings February 12, 2026 21:59

Copilot started reviewing on behalf of mvadari February 12, 2026 22:00 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

mvadari added 2 commits February 12, 2026 17:07

improve fail erroring

7e53b33

back out

de3a64c

Copilot AI review requested due to automatic review settings February 12, 2026 22:08

Copilot started reviewing on behalf of mvadari February 12, 2026 22:09 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

mvadari added 2 commits February 12, 2026 17:13

roll back more changes

322ac32

Revert "roll back test failures"

967bdc4

This reverts commit 78f11bc.

Copilot AI review requested due to automatic review settings February 12, 2026 22:17

Copilot started reviewing on behalf of mvadari February 12, 2026 22:18 View session

Copilot AI reviewed Feb 12, 2026

View reviewed changes

oops

4505091

	fail("Expected parse_error for mantissa larger than uint64 max");
	BEAST_EXPECTS(false, "Expected parse_error for mantissa larger than uint64 max");

		./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" 2>&1 \| tee unittest.log
		exit ${PIPESTATUS[0]}

-#include <source_location>
+#if defined(__has_include)
+# if __has_include(<source_location>)
+#  include <source_location>
+#  define BEAST_HAS_STD_SOURCE_LOCATION 1
+# endif
+#endif

	runner_->fail(reason);
	runner_->fail(detail::make_reason(reason, loc.file_name(), static_cast<int>(loc.line())));

	env(noop(alice), fee(env.balance(alice) - XRP(1)), ter(telINSUF_FEE_P));
	env(noop(alice), fee(env.balance(alice) - XRP(1)));

	BEAST_EXPECT(env.balance(alice) == XRP(1000));
	BEAST_EXPECT(env.balance(alice) == XRP(1));

chore: grep for failures in CI #6339

Are you sure you want to change the base?

chore: grep for failures in CI #6339

Conversation

mvadari commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High Level Overview of Change

Context of Change

Type of Change

API Impact

Test Plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

bthomee Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

mvadari Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mvadari commented Feb 6, 2026 •

edited

Loading

codecov bot commented Feb 6, 2026 •

edited

Loading