Remove file I/O from tests that don't need it #11182
Conversation
| continue; | ||
| .find(|term_str| contains_ignore_case(&val_str, term_str)) | ||
| else { | ||
| continue; |
There was a problem hiding this comment.
I think it's wrongly formatted, would you please run cargo fmt?
There was a problem hiding this comment.
well, that's a bit weird, but I did run cargo fmt:
There was a problem hiding this comment.
let ... = ... else {} formatting was only stabilized in very recent versions of rustfmt and may possibly still be treated preserved depending on the active toolchain version?
| let sample = r#" | ||
| [[name, rusty_luck, origin]; | ||
| [Jason, 1, Canada]] | ||
| "#; | ||
|
|
||
| let actual = nu!( | ||
| cwd: dirs.test(), pipeline( | ||
| let actual = nu!(pipeline(&format!( | ||
| " | ||
| mkdir temp; | ||
| cd temp; | ||
| print (open ../nu_times.csv).name.0 | table; | ||
| print (echo {sample}).name.0 | table; | ||
| cd ..; | ||
| rmdir temp | ||
| " | ||
| )); | ||
| ))); | ||
|
|
||
| // chop will remove the last escaped double quote from \"Estados Unidos\" | ||
| assert_eq!(actual.out, "Jason"); | ||
| }) | ||
| // chop will remove the last escaped double quote from \"Estados Unidos\" | ||
| assert_eq!(actual.out, "Jason"); |
There was a problem hiding this comment.
The title of the test suggests that this is to check that open ../nu_times.csv is properly handled.
So this probably has to be reverted.
| fn subexpression_handles_dot() { | ||
| Playground::setup("subexpression_handles_dot", |dirs, sandbox| { | ||
| sandbox.with_files(vec![FileWithContentToBeTrimmed( | ||
| "nu_times.csv", | ||
| " | ||
| name,rusty_luck,origin | ||
| Jason,1,Canada | ||
| JT,1,New Zealand | ||
| Andrés,1,Ecuador | ||
| AndKitKatz,1,Estados Unidos | ||
| ", | ||
| )]); | ||
|
|
||
| let actual = nu!( | ||
| cwd: dirs.test(), pipeline( | ||
| let sample = r#" | ||
| [[name, rusty_luck, origin]; | ||
| [Jason, 1, Canada], | ||
| [JT, 1, "New Zealand"], | ||
| [Andrés, 1, Ecuador], | ||
| [AndKitKatz, 1, "Estados Unidos"]] | ||
| "#; | ||
|
|
||
| let actual = nu!(pipeline(&format!( | ||
| " | ||
| echo (open nu_times.csv) |
There was a problem hiding this comment.
based on the title this sounds like it should check for (open nu_times.csv) being handled correctly.
Else the whole construct with echo (open ...) doesn't make much sense.
Maybe the rest of the test can be simplified to make it a bit less confusing but else we should preserve that based on whatever forced us to add it in the first place.
| {sample} | ||
| | wrap name | ||
| | default "arepa!" hit | ||
| | rename --column {} | ||
| | rename --column {{}} |
There was a problem hiding this comment.
Maybe it makes sense here to simply move the sample string into the pipe so we don't need the double {{}} here but that is just a stylistic minutia.
There was a problem hiding this comment.
I believe it is better to keep the formatting inside the pipeline, as the sample data contains multiple lines, and nu! treats all lines in its argument as separate commands.
# Description This PR implements modifications to command tests that write unnecessary json and csv to disk then load it with open, by using nuon literals instead. - Fixes nushell#7189 # User-Facing Changes None # Tests + Formatting This only affects existing tests, which still pass.
# Description This PR implements modifications to command tests that write unnecessary json and csv to disk then load it with open, by using nuon literals instead. - Fixes nushell#7189 # User-Facing Changes None # Tests + Formatting This only affects existing tests, which still pass.
Description
This PR implements modifications to command tests that write unnecessary json and csv to disk then load it with open, by using nuon literals instead.
User-Facing Changes
None
Tests + Formatting
This only affects existing tests, which still pass.
After Submitting
None