Repository navigation
revdep maditr affected #5128
Description
Activity
new revdep simDAG has a similar issue, git bisect says it started at the same time,
* checking tests ... Running 'testthat.R' ERROR Running the tests in 'tests/testthat.R' failed. Last 13 lines of output: 8. \-data.table:::gshift(start, type = "lead", fill = max_t) -- Failure ('test_sim2data_all_equal_last.r:35:3'): long: .all equal to .last -- `d_all` (`actual`) not equal to `d_last` (`expected`). `attr(actual, 'sorted')` is absent `attr(expected, 'sorted')` is a character vector ('.id', '.time') -- Failure ('test_sim2long.all.r:52:3'): general test case --------------------- `out_dat` (`actual`) not equal to `expected` (`expected`). `attr(actual, 'sorted')` is absent `attr(expected, 'sorted')` is a character vector ('.id', '.time') [ FAIL 5 | WARN 0 | SKIP 35 | PASS 367 ] Error: Test failures Execution haltedI believe we should keep this open until the revdep fixes this, or until it disappears from the revdep checker (currently still there) https://rcdata.nau.edu/genomic-ml/data.table-revdeps/analyze/2023-12-22/
Thanks @tdhock. The latest run still doesn't include the cited PR commit: 78dee17
This basically comes down to a difference of 'fixed' vs. 'fixed and verified' in this parlance: https://techcommunity.microsoft.com/t5/azure-devops/resolved-reason-field-in-azuredevops-difference-between-fixed/m-p/3816529
The PR fixed the issue, but until the revdeps check is cleared, it's not fully 'verified' (because we did not include a complete end-to-end replication of the package bug as a regression test).
I'm not sure we have a policy in place on this tracker for closing at 'fixed' vs. 'verified' -- we're in kind of new territory thanks to the revdep service you've set up. I'm happy to leave the bugs open until 'verified'.
- added a commit that references this issue
on Dec 23, 2023 Also filed a fix downstream: gdemin/maditr#17
Hi @tdhock, I still need help understanding the revdep results.
The Dec 23 results still show an error for {maditr} and apparently include the fix commit 78dee17:
https://rcdata.nau.edu/genomic-ml/data.table-revdeps/analyze/2023-12-23/
https://rcdata.nau.edu/genomic-ml/data.table-revdeps/analyze/2023-12-23/maditr.txtBut I'm unable to reproduce the error locally:
# in maditr cloned from cran/maditr on GitHub system2("R", c("CMD", "build", ".")) system2("R", c("CMD", "check", "maditr_*.tar.gz", "--as-cran")) # ... # Status: 1 WARNING packageVersion("maditr") # [1] ‘0.8.3’ read.dcf(system.file("DESCRIPTION", package="data.table"), "Revision") # Revision # [1,] "78dee17e647e16ccd23120594ed53d3d5934a87e"
(the
WARNINGis the expected warning from--as-cranabout mismatched author/version)thanks for your investigation Michael. Seems like there was a bug with the revdep checker. I had to look at the logs (private on NAU Monsoon) to figure out the issue. It is rather complicated but here goes. tl;dr it should be fixed now!
Before running revdep checks we need to build data.table master from source (R CMD build).
Part of that process is re-building vignettes, which recently changed from using rmarkdown to markdown (no r) for HTML vignettes.
There was logic to install rmarkdown, but no logic for installing markdown, so building failed. I installed markdown, and added logic for installing it again in the future if necessary, tdhock/data.table-revdeps@727a8c3
So the build failed, but since it was running via system("R CMD build ...") the R script keeps going even if that shell command fails.
So the installation and check was actually using the last version of master which had been built (without markdown dependency), but it reporting the current master sha hash.
Now to avoid such confusions in the future, I have added some logic to remove any previous data table tar gz builds, tdhock/data.table-revdeps@abb8b24 so the installation step will fail, rather than installing an incorrect previous version.
Phew! does that make sense to you?thanks for your investigation Michael. Seems like there was a bug with the revdep checker. I had to look at the logs (private on NAU Monsoon) to figure out the issue. It is rather complicated but here goes. tl;dr it should be fixed now! Before running revdep checks we need to build data.table master from source (R CMD build). Part of that process is re-building vignettes, which recently changed from using rmarkdown to markdown (no r) for HTML vignettes. There was logic to install rmarkdown, but no logic for installing markdown, so building failed. I installed markdown, and added logic for installing it again in the future if necessary, tdhock/data.table-revdeps@727a8c3 So the build failed, but since it was running via system("R CMD build ...") the R script keeps going even if that shell command fails. So the installation and check was actually using the last version of master which had been built (without markdown dependency), but it reporting the current master sha hash. Now to avoid such confusions in the future, I have added some logic to remove any previous data table tar gz builds, tdhock/data.table-revdeps@abb8b24 so the installation step will fail, rather than installing an incorrect previous version. Phew! does that make sense to you?
makes sense! great, I was also surprised not to see any failures related to logical01, I guess they'll come out in the next run too.
Reacted by Toby Dylan HockingIn .ci dir you have packages.dcf() which can resolve problem of changing dependencies
Now fixed+verified:
PS @tdhock another thing that might help would be to log the commit of the revdep code itself, to know what version of the code is being run (I couldn't tell yesterday whether the run included your fix or not).
Reacted by Toby Dylan Hockinggreat, I added the commit of data.table-revdeps code to the report, tdhock/data.table-revdeps@b5617e2
Last of the 4/1111 revdeps and it's the same as #5125, #5126 and #5127
Line 121 of test_take_all.R is :
which causes