Skip to content
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.

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

Show tests prints #422

Merged
merged 4 commits into from
Jun 12, 2020
Merged

Conversation

AbdouSeck
Copy link
Contributor

@AbdouSeck AbdouSeck commented Jun 4, 2020

This new feature can be accessed by invoking rustlings with --nocapture.

closes #262

MINOR CHANGES:
* Documentation added to source code under src
* Both unit and integration tests for --nocapture have been added.

BREAKING CHANGES:
The following function take a new boolean argument:
* run
* verify
* test
* compile_and_test

compile_and_test is the function that uses this new boolean argument to determine whether or not to display the output from the test harnesses.

Please feel free to reach out with any questions.

Thank you,

Abdou

This new feature can be accessed by invoking rustlings with --nocapture.

Both unit and integration tests added.

closes rust-lang#262

BREAKING CHANGES:
The following function take a new boolean argument:
	* `run`
	* `verify`
	* `test`
	* `compile_and_test`
@jrvidal
Copy link
Contributor

jrvidal commented Jun 4, 2020

ack, I'll look into this ASAP

Copy link
Contributor

@jrvidal jrvidal left a comment

Choose a reason for hiding this comment

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

LGTM

Ok(_) => {
Ok(output) => {
if verbose {
println!("{}", output.stdout);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm starting to think that we should be a bit more sophisticated while handling the output of exercises, and try to have both stdout and stderr merged as they would appear in the terminal screen.

Anyway, out of scope, just thinking out loud.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jrvidal thanks for the review! I agree with providing more information to the end users as long as there is a way to control that; like with command line options to make output --quiet or --verbose.

Normally, an exercise's standard error should be empty unless there is a complication error or a runtime one. Unless someone explicitly uses eprint! or eprintln! (or the many other ways of writing to targeted file handles) in their code to solve an exercise, there should be nothing in the standard error. So, keeping standard output and error separate may actually come in handy when trying to figure out if a user actually wrote to the standard error.

I am open to discussing this further, though.

Thanks!

Abdou

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm starting to think that we should be a bit more sophisticated while handling the output of exercises, and try to have both stdout and stderr merged as they would appear in the terminal screen.

Anyway, out of scope, just thinking out loud.

Done in the v6 branch with os_pipe :D

closes rust-lang#423

If the parsed JSON data curled during a bash installation is not valid, use the repository's tag files
as a backup. If those files don't exist somehow, then checkout the master branch and install it.
@AbdouSeck
Copy link
Contributor Author

I decided to a commit which hopefully closes #423. I just added a couple of steps to check for string emptiness and use master as the backup for installing the program when the latest tag can't be obtained.

Thanks,

Abdou

@AbdouSeck
Copy link
Contributor Author

@fmoko please let me know if this needs additional work to get it merged.

Thanks,

Abdou

@shadows-withal
Copy link
Member

@AbdouSeck All good! Just went under my radar.

@shadows-withal shadows-withal merged commit e1e4530 into rust-lang:master Jun 12, 2020
@AbdouSeck AbdouSeck deleted the show-tests-prints branch December 13, 2020 17:43
ppp3 pushed a commit to ppp3/rustlings that referenced this pull request May 23, 2022
dmoore04 pushed a commit to dmoore04/rustlings that referenced this pull request Sep 11, 2022
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.

View a specific exercise's stdout
5 participants