Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

output of test diffs gets overwritten #88

Closed
edzer opened this issue Oct 30, 2018 · 7 comments · Fixed by #95
Closed

output of test diffs gets overwritten #88

edzer opened this issue Oct 30, 2018 · 7 comments · Fixed by #95

Comments

@edzer
Copy link

edzer commented Oct 30, 2018

Great package!

If a package contains normal files in its tests directory, when they generate output, rcmdcheck seems to overprint this. Illustrated below. Package tested on: sf.
x

@gaborcsardi
Copy link
Member

This has to fixed upstream first, the problem is that callr::rcmd mixes stdout and stderr, and R prints the test output on stderr.

@eddelbuettel
Copy link

That may be my issue too -- having used this for years from littler via the rcc.r script I am clearly seeing formatting / line erase artefacts for tests. I reckon that I am not your typical use case (as it is not devtools or RStudio) but if that could get fixed I'd appreciate it. Eg here is a test I just ran on Rcpp and there are eight vignettes squashed into one line:

image

It all works, and is both functional and pretty ... just not as pretty as it could be.

@gaborcsardi
Copy link
Member

We can definitely improve this, and will soonish, but it is not as easy as it looks. The reason is that R prints the test output to stderr, and the rest of the output to stdout. We capture them separately, and then the timing information is basically lost, and we print them intermingled.

One way to solve this is to send both stdout and stderr into the same pipe/socket, but callr cannot do that currently AFAICT. Another way is to only print stderr, if it is a complete line, that'll complicate my output parsing a bit. Anyway, will fix soon.

@eddelbuettel
Copy link

Your call. But if it can't be done nicely, then maybe don't do it. Even a small, but consistent and predictable, amount of such "line noise" takes away from the work done making the rest pretty.

Anyway, as long as you are aware...

@gaborcsardi
Copy link
Member

gaborcsardi commented Dec 2, 2018

OK, while timely stdout & stderr collection is a problem, it seems that in this particular case the main problem is just not handling the output of the .Rout comparison correctly. So I can fix this in rcmdcheck relatively easily.

One question is what to do with output differences. R CMD check does not even give a NOTE for these, but it does print them, so we should probably include them in the returned object.

@jeroen
Copy link
Member

jeroen commented Dec 2, 2018

One question is what to do with output differences. R CMD check does not even give a NOTE for these, but it does print them, so we should probably include them in the returned object.

The spelling package uses this to print potential spelling errors, without causing an actual check NOTE or ERROR. The diff output is often intended for manual visual inspection, so I think it should somehow be available in the return object.

gaborcsardi added a commit that referenced this issue Dec 3, 2018
For tests that compare the output. Closes #88.
@eddelbuettel
Copy link

I am current with released packages and I am still seeing the issue, so not sure closing is the right move.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants