yihui / xfun

Miscellaneous R functions
https://yihui.org/xfun/
Other
134 stars 27 forks source link

Regression detected with v0.44 #85

Closed CGMossa closed 3 months ago

CGMossa commented 3 months ago

By filing an issue to this repo, I promise that

I understand that my issue may be closed if I don't fulfill my promises.

There is a regression occurring with the recently released v0.44. We use snapshot tests on a knitr-powered document in one of our CI runs, and it is failing with the new xfun version.

An example of the output, which is also showing the issue here in the details slot. This comes as a result of the snapshot testing. This works perfectly fine with previous versions of xfun.

Details

``` Failure (test-knitr-engine.R:33:3): Snapshot test of knitr-engine Snapshot of code has changed: old | new [7] Basic use example: | Basic use example: [7] [8] | [8] [9] | [9] [10] ```r - ``` r [10] [11] library(rextendr) | library(rextendr) [11] [12] | [12] [13] # create a Rust function | # create a Rust function [13] old[33:39] vs new[33:39] would create the following output in the knitted document: - ```rust + ``` rust rprintln!("Hello from Rust!"); let x = 5; old | new [48] Define variable `_x`: | Define variable `_x`: [48] [49] | [49] [50] | [50] [51] ```rust - ``` rust [51] [52] let _x = 1; | let _x = 1; [52] [53] ``` | ``` [53] [54] | [54] old | new [55] Define variable `_y`: | Define variable `_y`: [55] [56] | [56] [57] | [57] [58] ```rust - ``` rust [58] [59] let _y = 2; | let _y = 2; [59] [60] ``` | ``` [60] [61] | [61] old[62:74] vs new[62:74] Print: - ```rust + ``` rust rprintln!("x = {}, y = {}", _x, _y); #> x = 1, y = 2 ``` - ```rust + ``` rust and 3 more ... old[83:89] vs new[83:89] ``` - ```r + ``` r md_text <- "# The story of the fox The quick brown fox **jumps over** the lazy dog. The quick *brown fox* jumps over the lazy dog." ```

For extensive outputs from the CI run see: https://github.com/extendr/extendr/actions/runs/9097328697/job/25004901212?pr=762

I've also tested this locally. And if I specifically update to this new version, I get the same error

Running xfun::session_info("xfun") results in

> xfun::session_info("xfun")
Error: lazy-load database '/Library/Frameworks/R.framework/Versions/4.4-arm64/Resources/library/xfun/R/xfun.rdb' is corrupt
In addition: Warning message:
internal error -3 in R_decompress1 

Maybe this is caused by restarting R, maybe it is an issue, but if I restart R, I get this output

> xfun::session_info("xfun")
R version 4.4.0 (2024-04-24)
Platform: aarch64-apple-darwin20
Running under: macOS Sonoma 14.4.1

Locale: en_US.UTF-8 / en_US.UTF-8 / en_US.UTF-8 / C / en_US.UTF-8 / en_US.UTF-8

Package version:
  graphics_4.4.0  grDevices_4.4.0 stats_4.4.0     tools_4.4.0    
  utils_4.4.0     xfun_0.44 

Update: I think it is relevant to have this session info as well

> xfun::session_info("knitr")
R version 4.4.0 (2024-04-24)
Platform: aarch64-apple-darwin20
Running under: macOS Sonoma 14.4.1

Locale: en_US.UTF-8 / en_US.UTF-8 / en_US.UTF-8 / C / en_US.UTF-8 / en_US.UTF-8

Package version:
  evaluate_0.23   graphics_4.4.0  grDevices_4.4.0 highr_0.10     
  knitr_1.46      methods_4.4.0   stats_4.4.0     tools_4.4.0    
  utils_4.4.0     xfun_0.44       yaml_2.3.8 
CGMossa commented 3 months ago

Okay, I'm almost sure that this is related to https://github.com/yihui/xfun/commit/3e134c572d643c11ad3a285e1bee18bdc412fd9d . And the spaces I see in my test, shows up here https://github.com/yihui/knitr/commit/efd6aa02954a2f82fc2a82f0cc7b6508fde8b9c2.

I'm now confident that I just need to know how come this is a valid way to annotate code-fences.. Sorry about that, this might be completely intentional and not a regression

yihui commented 3 months ago

Sorry for the trouble! The change was intentional. I've adjusted my snapshot tests accordingly: https://github.com/yihui/knitr-examples/commit/931e0a2a4e273dcdc13902291b6c6b6e448658cb

I just need to know how come this is a valid way to annotate code-fences..

It is valid. The leading space is optional: https://github.github.com/gfm/#fenced-code-blocks

The line with the opening code fence may optionally contain some text following the code fence; this is trimmed of leading and trailing whitespace and called the info string

CGMossa commented 3 months ago

Thanks! I think we have everything we need here. This was very pleasant experience, thanks again!

yihui commented 3 months ago

You are welcome!

BTW, Rust and extendr are high on my learning list, so thank you and the team for the great effort! I'm highly interested in learning them, although I'm very weak at learning new languages.

eitsupi commented 3 months ago

Hi, I am confused as the test seems to be failing on CRAN even though there is 3e134c572d643c11ad3a285e1bee18bdc412fd9d. If I understand correctly, 3e134c572d643c11ad3a285e1bee18bdc412fd9d is supposed to maintain conventional behavior on the old knitr, but it doesn't actually work?

https://cran.r-project.org/web/checks/check_results_prqlr.html

Version: 0.8.0
Check: tests
Result: ERROR
    Running ‘testthat.R’ [8s/10s]
  Running the tests in ‘tests/testthat.R’ failed.
  Complete output:
    > # This file is part of the standard setup for testthat.
    > # It is recommended that you do not modify it.
    > #
    > # Where should you do additional test configuration?
    > # Learn more about the roles of various files in:
    > # * https://r-pkgs.org/tests.html
    > # * https://testthat.r-lib.org/reference/test_package.html#special-files
    > 
    > library(testthat)
    > library(prqlr)
    > 
    > test_check("prqlr")
    [ FAIL 7 | WARN 0 | SKIP 17 | PASS 13 ]

    ══ Skipped tests (17) ══════════════════════════════════════════════════════════
    • On CRAN (17): 'test-compile.R:60:3', 'test-compile.R:74:5',
      'test-compile.R:74:5', 'test-compile.R:74:5', 'test-compile.R:94:5',
      'test-compile.R:94:5', 'test-compile.R:94:5', 'test-compile.R:94:5',
      'test-compile.R:94:5', 'test-compile.R:94:5', 'test-compile.R:94:5',
      'test-compile.R:94:5', 'test-compile.R:94:5', 'test-compile.R:94:5',
      'test-compile.R:94:5', 'test-compile.R:94:5', 'test-compile.R:100:3'

    ══ Failed tests ════════════════════════════════════════════════════════════════
    ── Failure ('test-knitr-engine.R:22:3'): Snapshot test of knitr-engine ─────────
    Snapshot of code has changed:
         old                 | new                     
     [4]                     |                     [4] 
     [5]                     |                     [5] 
     [6]                     |                     [6] 
     [7]   ```elm            -   ``` elm           [7] 
     [8]   from mtcars       |   from mtcars       [8] 
     [9]   filter cyl > 6    |   filter cyl > 6    [9] 
    [10]   select {cyl, mpg} |   select {cyl, mpg} [10]

         old                 | new                     
    [31]   ```               |   ```               [31]
    [32]                     |                     [32]
    [33]                     |                     [33]
    [34]   ```elm            -   ``` elm           [34]
    [35]   from mtcars       |   from mtcars       [35]
    [36]   filter cyl > 6    |   filter cyl > 6    [36]
    [37]   select {cyl, mpg} |   select {cyl, mpg} [37]
yihui commented 2 months ago

@eitsupi 3e134c5 was only for knitr and not for packages that depend on knitr.

I wonder if CRAN has contacted you to fix this problem. I can fix it on knitr's end, but if CRAN has reached out to package authors and they have started adjusting their tests, it would be too late for me to fix it, otherwise the adjusted tests would be broken again after I release a new version of knitr to CRAN...

eitsupi commented 2 months ago

I see, I am convinced. Too late since CRAN informed me last week and I have already released a new version.

yihui commented 2 months ago

Okay, in that case, my apologies for the hassle!