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

Fix test for {renv} #486

Merged
merged 6 commits into from
Jul 7, 2023
Merged

Fix test for {renv} #486

merged 6 commits into from
Jul 7, 2023

Conversation

zkamvar
Copy link
Contributor

@zkamvar zkamvar commented Jun 30, 2023

This fixes a single test for {renv} due to a difference in output it created.

Note: because this only affects one test and the devel version of {renv}, the new version does not need to be released after this is merged.

This will fix #484

zkamvar added 2 commits June 30, 2023 12:11
I also fixed the date on the release and added headings.
@zkamvar zkamvar requested a review from a team June 30, 2023 19:16
Copy link
Contributor

@froggleston froggleston left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@zkamvar zkamvar enabled auto-merge June 30, 2023 19:24
@zkamvar zkamvar disabled auto-merge June 30, 2023 19:25
@zkamvar
Copy link
Contributor Author

zkamvar commented Jun 30, 2023

Now it appears that there is a problem with R-devel and the internal .make_numeric_version() throwing a new warning, which was introduced into devel R two days ago: wch/r-source@1338a95

We are now inundated with new warnings in R-devel and it's changing our snapshot tests. I believe the solution is to use the devel version of {rmarkdown} for our tests. The solution is to fix the 0 here to be "0":

if (pan$version > 0) {

Condition
  Warning in `.make_numeric_version()`:
  invalid non-character version specification 'x' (type: double)

I suspect this is actually coming from {rmarkdown} because this commit (rstudio/rmarkdown@01a1737) references https://bugs.r-project.org/show_bug.cgi?id=18548, which makes this comparison illegal:

numeric_version("1.5") == 1.5
#> [1] TRUE

Created on 2023-06-30 with reprex v2.0.2

Warnings

── Failure ('test-check_pandoc.R:12:3'): check_pandoc() throws a message about installation [ansi] ──
Snapshot of code has changed:
old[1:5] vs new[1:8]
  Code
    expect_error(check_pandoc(pv = "42"), "Incorrect pandoc version")
+ Condition
+   Warning in `.make_numeric_version()`:
+   invalid non-character version specification 'x' (type: double)
  Message
     sandpaper requires pandoc version 42 or higher.
    ! You have pandoc version [version masked for testing] in '[path masked for testing]'

* Run `testthat::snapshot_accept('check_pandoc')` to accept the change.
* Run `testthat::snapshot_review('check_pandoc')` to interactively review the change.
── Failure ('test-check_pandoc.R:12:3'): check_pandoc() throws a message about installation [unicode] ──
Snapshot of code has changed:
old[1:5] vs new[1:8]
  Code
    expect_error(check_pandoc(pv = "42"), "Incorrect pandoc version")
+ Condition
+   Warning in `.make_numeric_version()`:
+   invalid non-character version specification 'x' (type: double)
  Message
     sandpaper requires pandoc version 42 or higher.
    ! You have pandoc version [version masked for testing] in '[path masked for testing]'

* Run `testthat::snapshot_accept('check_pandoc')` to accept the change.
* Run `testthat::snapshot_review('check_pandoc')` to interactively review the change.
── Failure ('test-check_pandoc.R:12:3'): check_pandoc() throws a message about installation [fancy] ──
Snapshot of code has changed:
old[1:5] vs new[1:8]
  Code
    expect_error(check_pandoc(pv = "42"), "Incorrect pandoc version")
+ Condition
+   Warning in `.make_numeric_version()`:
+   invalid non-character version specification 'x' (type: double)
  Message
     sandpaper requires pandoc version 42 or higher.
    ! You have pandoc version [version masked for testing] in '[path masked for testing]'

* Run `testthat::snapshot_accept('check_pandoc')` to accept the change.
* Run `testthat::snapshot_review('check_pandoc')` to interactively review the change.
── Failure ('test-check_pandoc.R:18:3'): check_pandoc throws a message about installation for RStudio [plain] ──
Snapshot of code has changed:
old[1:6] vs new[1:9]
  Code
    expect_error(check_pandoc(pv = "42", rv = "94"), "Incorrect pandoc version",
    fixed = TRUE)
+ Condition
+   Warning in `.make_numeric_version()`:
+   invalid non-character version specification 'x' (type: double)
  Message
     sandpaper requires pandoc version 42 or higher.
    ! You have pandoc version [version masked for testing] in '[path masked for testing]'

* Run `testthat::snapshot_accept('check_pandoc')` to accept the change.
* Run `testthat::snapshot_review('check_pandoc')` to interactively review the change.
── Failure ('test-check_pandoc.R:18:3'): check_pandoc throws a message about installation for RStudio [ansi] ──
Snapshot of code has changed:
old[1:6] vs new[1:9]
  Code
    expect_error(check_pandoc(pv = "42", rv = "94"), "Incorrect pandoc version",
    fixed = TRUE)
+ Condition
+   Warning in `.make_numeric_version()`:
+   invalid non-character version specification 'x' (type: double)
  Message
     sandpaper requires pandoc version 42 or higher.
    ! You have pandoc version [version masked for testing] in '[path masked for testing]'

* Run `testthat::snapshot_accept('check_pandoc')` to accept the change.
* Run `testthat::snapshot_review('check_pandoc')` to interactively review the change.
── Failure ('test-check_pandoc.R:18:3'): check_pandoc throws a message about installation for RStudio [unicode] ──
Snapshot of code has changed:
old[1:6] vs new[1:9]
  Code
    expect_error(check_pandoc(pv = "42", rv = "94"), "Incorrect pandoc version",
    fixed = TRUE)
+ Condition
+   Warning in `.make_numeric_version()`:
+   invalid non-character version specification 'x' (type: double)
  Message
     sandpaper requires pandoc version 42 or higher.
    ! You have pandoc version [version masked for testing] in '[path masked for testing]'

* Run `testthat::snapshot_accept('check_pandoc')` to accept the change.
* Run `testthat::snapshot_review('check_pandoc')` to interactively review the change.
── Failure ('test-check_pandoc.R:18:3'): check_pandoc throws a message about installation for RStudio [fancy] ──
Snapshot of code has changed:
old[1:6] vs new[1:9]
  Code
    expect_error(check_pandoc(pv = "42", rv = "94"), "Incorrect pandoc version",
    fixed = TRUE)
+ Condition
+   Warning in `.make_numeric_version()`:
+   invalid non-character version specification 'x' (type: double)
  Message
     sandpaper requires pandoc version 42 or higher.
    ! You have pandoc version [version masked for testing] in '[path masked for testing]'

* Run `testthat::snapshot_accept('check_pandoc')` to accept the change.
* Run `testthat::snapshot_review('check_pandoc')` to interactively review the change.

@zkamvar zkamvar requested a review from froggleston June 30, 2023 19:45
DESCRIPTION Outdated Show resolved Hide resolved
@zkamvar
Copy link
Contributor Author

zkamvar commented Jun 30, 2023

@carpentries/workbench-maintainers, please review (and subsequently merge if it looks cromulent) this when you get the chance.

@zkamvar zkamvar requested review from a team and removed request for froggleston June 30, 2023 20:25
@froggleston
Copy link
Contributor

froggleston commented Jul 5, 2023

I completely agree that this is weird. The function is make_numeric_version but expects a char as input. Strange. They check for ischaracter() so why not also check for isnumeric() and support both? shrugs (edit: hmm I guess 1.3.5 for example isn't numeric so would need to be a char at any rate)

@zkamvar zkamvar merged commit cb3d6c4 into main Jul 7, 2023
@zkamvar zkamvar deleted the fix-renv-test-484 branch July 7, 2023 16:42
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.

devel {renv} changed its messaging again, breaking one test
2 participants