-
Notifications
You must be signed in to change notification settings - Fork 105
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
Tests break on Windows #101
Comments
Turns out it less dramatic. Win-builder only broke on r-devel, maybe for lack of git2r. It was a simple issue of checking the return code of the first test function. Should now be better. |
I tested on Windows and Linux, but on r-release. So it should work. Do you have a link to a broken build report, because the last ones here on Github are fine, aren't they? |
Yep. Windows r-devel came back first and "surprised me". r-release was fine. Logs from r-devel are below but I should have a fix for this and am just waiting for a second run. Dear package maintainer, this notification has been generated automatically. All the best, |
Ok let me know. I have r-devel on Windows set up in a VM... |
Thanks for the offer. Dear package maintainer, this notification has been generated automatically. All the best, The one nag is the usual "yo you hit your GH API hits limit" following links to GitHub URLs. I think I'll ship 0.1.7 -- thanks again for all your patient help! |
It still breaks on r-devel, so I finally put a > cat("****** repodir is: '", repodir, "'\n", sep="")
****** repodir is: 'D:\temp\RtmpY3pa34/working_dir\Rtmpq8vS13/drat'
> if (length(repodir) > 0L) testRepoActions(repodir)
Error in testRepoActions(repodir) : Wrong package files found
Execution halted I don't really have access to Windows. Do you think you can maybe take a stab at this? I suspect it fails over the non-standard forward and backward slashes. Edit: I gave it a shot but had bad luck with win-builder which seems to have it a pothole as it doesn't now and again. Edit 2: And I guess thinks don't "generally" break but under "some situation" -- maybe once we cross drives on Windows as above with |
I will have a look at it later. edit: do you have a revision number for r-devel? |
Thanks for looking into it. We probably want to cover this is several spots whereever pieces of a path are computed or recalled. As for the revision number: what do you mean? R-devel I don't control, I just submit to the builder. For the local tarball I was optimisitic earlier and thought I was near a release so I had 0.1.7. I then needed to differentiate between runs hence 0.1.7.0.x, and then realized > 0.1.7 was a bad idea. In short, it is "just" in the local DESCRIPTION so far. |
I guess the only place a path could go wrong for Line 291 in 499b964
a call to |
That's a good one, trying that. Something is a corner case anyway because "some" of the machines build (rhub, win-builder r-release) and only some do not (win-builder r-devel) so it seems input-dependent too, i.e. we still have a buglet somewhere not cleaning an input value from somewhere. |
From previous experience, it sounds like a bug in an r revision. I had the problem, when updating So to recap: On CRAN it works for 4.0.2 (#101 (comment)) but not for r78775 (#101 (comment)). So what version of r-devel is rhub running? If it is >r78775, I would guess the error was fixed in later revisions (Current is r78794) and the builds on CRAN should clear hopefully. edit: can you link the output of the rhub builds? (Never used it, so I don't if that is an option) |
Err, hold on. I never laid blame on r-devel. I have r-devel here. I have seen repeated predictable errors clearly blaming us for generating non-portable paths. I think you are extrapolating. The main problem right now is that the box I could produce errors on --- r-devel at win-builder --- appears to not run now. |
🤦🤦🤦 versions for the tests were not fixed. See #102 |
And whooooopsies. Looks like I didn't test too well on r-devel here either. (Oh, now I now: my Thanks for cleaning up here! I may not get to it for the rest of the day but hopefully tomorrow... |
(Do we then have to always fix the test versions to "actual" values of R current, past, next, ... ?) |
Nope, shouldn't be the case now. I checked all the function calls and the ones who need it have a fixed version statement. |
And tests came back late yesterday so I'll fold this in now, look it over one more and then we're good too. Penny also dropped that the "version" here is driven by the file not the R moving the file around. Doh. |
Fixed in #102. |
@FelixErnst I was just about to wrap this up for a 0.1.7 release when I noticed that ... Windows tests are not happy.
I think I would be happy to just skip tests on Windows. Do you want to take a deeper dive? I keep forgetting if you mostly live on macOS or Linux, or Windows. Any inisghts?
I can share the win-builder links which will be good another day or two or so...
The text was updated successfully, but these errors were encountered: