Skip to content

Commit

Permalink
Ensure title exported in SCRUT_TEST contains only printable characters
Browse files Browse the repository at this point in the history
Summary:
This CL changes the value of SCRUT_TEST to contain <file-name>:<line-number>

# What?

- The value of `SCRUT_TEST` now contains the file-name followed by the line number (e.g. `some/test.md:123`) instead of the file-name followed by the title

# Why?

The filtering that Scrut uses to make sure some internal variables are passed between executions only works with single-line values. The newly introduced SCRUT_TEST variable was added to that filter list, but it could contain multi-line values (i.e. when a Scrut test title contained multiple lines). This resulted in invalid `state` files that broke subsequent Scrut tests (see failing tests in MSDK, e.g. D60034934)

Reviewed By: abesto

Differential Revision: D60039617

fbshipit-source-id: e99279c250e8a711d4c51f99c12e1120d702a329
  • Loading branch information
ukautz authored and facebook-github-bot committed Jul 22, 2024
1 parent 258826c commit cf252e3
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 3 deletions.
2 changes: 1 addition & 1 deletion selftest/cases/environment.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,5 +74,5 @@ GREP_OPTIONS: ''

```scrut
$ echo "SCRUT_TEST: $SCRUT_TEST"
SCRUT_TEST: .*[/\\]selftest[/\\]cases[/\\]environment\.md::Haz SCRUT_TEST (regex)
SCRUT_TEST: .*[/\\]selftest[/\\]cases[/\\]environment\.md:76 (regex)
```
6 changes: 5 additions & 1 deletion src/executors/stateful_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,11 @@ impl Executor for StatefulExecutor {
testcase.config.timeout = timeout;
testcase.config.environment.insert(
"SCRUT_TEST".into(),
format!("{}::{}", context.file.to_string_lossy(), testcase.title),
format!(
"{}:{}",
context.file.to_string_lossy(),
testcase.line_number
),
);

// run the execution, using the shared state directory
Expand Down
2 changes: 1 addition & 1 deletion website/docs/advanced/specifics.md
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ Scrut sets a list of environment variables for the execution. These are set _in
- `TESTFILE`: contains the name of the file that contains the test that is currently being executed
- `TESTSHELL`: contains the shell that in which the test is being executed in (default `/bin/bash`, see `--shell` flag on commands)
- `TMPDIR`: contains the absolute path to a temporary directory that will be cleaned up after the test is executed. This directory is shared in between all executed tests across all test files.
- `SCRUT_TEST`: contains the path to the test and the title, separated by double colons (e.g. `some/test.md::Some Title`). *This variable is recommend to use when deciding whether an execution is within Scrut.* Note: **the title is provided as given and therefore can contain spaces**!
- `SCRUT_TEST`: contains the path to the test and the line number, separated by a colon (e.g. `some/test.md:123`). *This variable is recommend to use when deciding whether an execution is within Scrut.* Note: **the title is provided as given and therefore can contain spaces**!

### Common (linux) environment variables

Expand Down

0 comments on commit cf252e3

Please sign in to comment.