Skip to content

feat(test): show executed commands for failing test scripts#2266

Open
mohitdebian wants to merge 8 commits intoprefix-dev:mainfrom
mohitdebian:fix-test-script-logging
Open

feat(test): show executed commands for failing test scripts#2266
mohitdebian wants to merge 8 commits intoprefix-dev:mainfrom
mohitdebian:fix-test-script-logging

Conversation

@mohitdebian
Copy link
Copy Markdown
Contributor

Fixes #2264

Currently rattler-build only reports "Script failed with status 1" when a
tests.script command fails, which makes it difficult to determine which
command caused the failure.

This change prepends set -x to test scripts in run_commands_test()
so that each command is printed before execution.

The change only affects test scripts and does not modify build script behavior.

@wolfv
Copy link
Copy Markdown
Member

wolfv commented Mar 10, 2026

Can we change this to always use -x?

@mohitdebian mohitdebian force-pushed the fix-test-script-logging branch from afd64fb to 1e03759 Compare March 11, 2026 05:17
@mohitdebian
Copy link
Copy Markdown
Contributor Author

Can we change this to always use -x?

Yes updated the implementation accordingly instead of injecting set -x into the script the bash interpreter now runs with -x when executing test scripts.

@mohitdebian
Copy link
Copy Markdown
Contributor Author

mohitdebian commented Mar 15, 2026

@wolfv I updated the implementation based on your suggestions:

instead of injecting set -x the bash interpreter now runs scripts with -x the change has also been moved to the interpreter layer so it applies to all scripts generated by rattler-build including both build scripts and test scripts

CI checks are running now please let me know if any further adjustments are needed.

@mohitdebian mohitdebian force-pushed the fix-test-script-logging branch from bb32ffb to 9baab05 Compare March 15, 2026 18:44

// Clone the script and prepend `set -x` to enable bash tracing for test scripts.
// Only inject for bash/sh scripts (interpreter unset defaults to bash on unix).
let mut test_script = commands_test.script.clone();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this can all be removed now, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes that block has been removed in the updated pr since tracing is now handled at the interpreter level via bash -x the set -x injection logic in run_commands_test() is no longer needed

@mohitdebian mohitdebian force-pushed the fix-test-script-logging branch from 5d959a9 to 5fba653 Compare March 17, 2026 19:16
@mohitdebian
Copy link
Copy Markdown
Contributor Author

@wolfv quick ping I’ve addressed the requested changes and removed the obsolete logic. CI is passing now would appreciate another look when you have time

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

no visibility into failing script: calls during test

2 participants