Skip to content

chore: grep for failures in CI #6339

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.

Sign up for GitHub

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

Merged
bthomee merged 24 commits into from
Feb 27, 2026
+18 −5
Merged

chore: grep for failures in CI #6339

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
2d529a2
grep for failures
mvadari Feb 6, 2026
22a25cf
add test failure
mvadari Feb 6, 2026
3fddd79
another failure test
mvadari Feb 6, 2026
82d7645
simplify output
mvadari Feb 6, 2026
78f11bc
roll back test failures
mvadari Feb 6, 2026
1294aac
fix parse failure for real
mvadari Feb 6, 2026
7e804a5
change string
mvadari Feb 6, 2026
d4bc64b
Merge remote-tracking branch 'upstream/develop' into better-erroring
mvadari Feb 10, 2026
4d385e9
better exit code
mvadari Feb 12, 2026
8760f80
Merge branch 'develop' into better-erroring
mvadari Feb 12, 2026
7e53b33
improve fail erroring
mvadari Feb 12, 2026
de3a64c
back out
mvadari Feb 12, 2026
322ac32
roll back more changes
mvadari Feb 12, 2026
967bdc4
Revert "roll back test failures"
mvadari Feb 12, 2026
4505091
oops
mvadari Feb 12, 2026
ea61d6d
Merge remote-tracking branch 'upstream/develop' into better-erroring
mvadari Feb 13, 2026
2bae8ee
Merge branch 'develop' into better-erroring
mvadari Feb 18, 2026
c6cfdf1
undo error
mvadari Feb 18, 2026
eceb5fc
Merge remote-tracking branch 'upstream/develop' into better-erroring
mvadari Feb 24, 2026
6964335
fix pre-commit
mvadari Feb 24, 2026
715916e
Merge branch 'develop' into better-erroring
mvadari Feb 24, 2026
93d0e12
Merge branch 'develop' into better-erroring
mvadari Feb 26, 2026
9028c2e
Merge branch 'develop' into better-erroring
mvadari Feb 26, 2026
404451a
Update .github/workflows/reusable-build-test-config.yml
mvadari Feb 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .github/workflows/reusable-build-test-config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,14 @@ jobs:
env:
BUILD_NPROC: ${{ steps.nproc.outputs.nproc }}
run: |
./xrpld --unittest --unittest-jobs "${BUILD_NPROC}"
./xrpld --unittest --unittest-jobs "${BUILD_NPROC}" 2>&1 | tee unittest.log
exit ${PIPESTATUS[0]}
Copy link
Collaborator

@bthomee bthomee Feb 9, 2026

Choose a reason for hiding this comment

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

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]}

Copy link
Collaborator Author

@mvadari mvadari Feb 12, 2026

Choose a reason for hiding this comment

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

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


- name: Show test failure summary
Comment on lines +232 to +235
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

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).

Copilot uses AI. Check for mistakes.
if: ${{ failure() && !inputs.build_only }}
working-directory: ${{ runner.os == 'Windows' && format('{0}/{1}', env.BUILD_DIR, inputs.build_type) || env.BUILD_DIR }}
run: |
grep -E "failed" unittest.log || echo "No failure details found in log"

- name: Debug failure (Linux)
if: ${{ failure() && runner.os == 'Linux' && !inputs.build_only }}
Expand Down
9 changes: 4 additions & 5 deletions src/test/app/Vault_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4973,20 +4973,19 @@ class Vault_test : public beast::unit_test::suite
env.close();

// 2. Mantissa larger than uint64 max
env.set_parse_failure_expected(true);
try
{
tx[sfAssetsMaximum] = "18446744073709551617e5"; // uint64 max + 1
env(tx, THISLINE);
BEAST_EXPECT(false);
fail("Expected parse_error for mantissa larger than uint64 max");
Copy link

Copilot AI Feb 6, 2026

Choose a reason for hiding this comment

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

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");

Copilot uses AI. Check for mistakes.
}
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);
Comment on lines +5343 to +5356
Copy link

Copilot AI Feb 12, 2026

Choose a reason for hiding this comment

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

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 uses AI. Check for mistakes.
}
}

Expand Down