Function Network Test Failure for Only milne test package on devel (does not fail on release)

[bburns632]

Documenting this one weird test failure on devel that unfortunately still occurs after current changes in PR #318. Appreciate input before diving that much deeper.

Context:
When running our (fancy new) release workflow, we get these unit test failures:

1. A bunch of checkRd: (-1) FunctionReporter.Rd:98-102: Lost braces in \itemize; meant \describe ? type failures. This indicates to me that accommodation for the Roxygen2 DESCRIPTION file "Roxygen: list(r6 = FALSE)" workaround is being dropped in upcoming R versions 😢 .
2. Just one failure in the function network for milne, our test package for exclusively R6 object packages. For that, the log is as follows:

══ Failed tests ════════════════════════════════════════════════════════════════
── Error ('test-FunctionReporter-network.R:20:9'): FunctionReporter extracts expected network for milne ──
<subscriptOutOfBoundsError/error/condition>
Error in `xList[[1]]`: subscript out of bounds
Backtrace:
     ▆
  1. ├─testthat::expect_equivalent(...) at test-FunctionReporter-network.R:20:9
  2. │ └─testthat::quasi_label(enquo(object), label, arg = "object")
  3. │   └─rlang::eval_bare(expr, quo_get_env(quo))
  4. └─pkgnet (local) `<fn>`()
  5.   └─private$extract_edges()
  6.     ├─data.table::rbindlist(...)
  7.     └─base::mapply(...)
  8.       └─pkgnet (local) `<fn>`(...)
  9.         ├─data.table::data.table(SYMBOL = unique(.parse_R6_expression(mbody)))
 10.         ├─base::unique(.parse_R6_expression(mbody))
 11.         └─pkgnet:::.parse_R6_expression(mbody)
 12.           ├─base::unlist(lapply(x, .parse_R6_expression), use.names = FALSE)
 13.           └─base::lapply(x, .parse_R6_expression)
 14.             └─pkgnet (local) FUN(X[[i]], ...)
 15.               ├─base::unlist(lapply(x, .parse_R6_expression), use.names = FALSE)
 16.               └─base::lapply(x, .parse_R6_expression)
 17.                 └─pkgnet (local) FUN(X[[i]], ...)

[.... and so on ....]

29.                                 └─pkgnet (local) FUN(X[[i]], ...)

[ FAIL 1 | WARN 0 | SKIP 0 | PASS 279 ]
Error: Error: Test failures
teardown-setTestEnv.R | NOT_CRAN = true 
Execution halted

Assumption:
They are related failures. Why?

• We made changes to .parse_function but not .parse_R6_expression which is being called here.
• Other, non-R6 exclusive, function networks are not failing tests.
• We are not doing R6 documentation up to current standards

Plan:

1. ✅ Update all test package DESCRIPTION files to align with Roxygen settings that ignore standard R6 object documentation. (Didn't work 😢 )
2. See if I'm missing something in the unit test error (like why we might have a valid issue with that function). If true, may still be able to punt R6 documentation re-write to the future. 📒
3. (Most likely) Need to re-write all roxygen documentation to meet now standard formatting. 😩

Any thoughts @jayqi ?

[bburns632]

Here's context on the attempted workaround and current syntax: https://roxygen2.r-lib.org/articles/rd-other.html#r6

[bburns632]

I updated milne docs and this error persists. No fix there. Need to debug function itself.

[bburns632]

Update: seems to failing within the remap_func_envs function referenced within deep_clone. There was an update to R6::deep_clone last year and it is in dev version. Looking into it as time allows.

https://github.com/r-lib/R6/blob/main/NEWS.md

Fixed #253: Errors could occur when deep cloning if a member object was an environment with a class that had a $ method. Deep cloning now uses get0() instead of $. R6 now requires R >= 3.2. (@zeehio, #274)

r-lib/R6#247

https://github.com/r-lib/R6/blob/507867875fdeaffbe7f7038291256b798f6bb042/R/clone.R#L12-L27

r-lib/R6@4dba01a

[bburns632]

Ultimately, that code as written in R6, when parsed with our parsing logic in pkgnet, results in an empty list that breaks our recursion. Recursion is updated in #322 to be tolerant to empty lists.

[zeehio]

Update: seems to failing within the remap_func_envs function referenced within deep_clone. There was an update to R6::deep_clone last year and it is in dev version. Looking into it as time allows.

https://github.com/r-lib/R6/blob/main/NEWS.md

Fixed #253: Errors could occur when deep cloning if a member object was an environment with a class that had a $ method. Deep cloning now uses get0() instead of $. R6 now requires R >= 3.2. (@zeehio, #274)

r-lib/R6#247

https://github.com/r-lib/R6/blob/507867875fdeaffbe7f7038291256b798f6bb042/R/clone.R#L12-L27

r-lib/R6@4dba01a

Hi,

I authored that change and I've been following this thread. Unfortunately I fail to see how my change was related to this issue.

It seems there is a PR with a possible fix on the pkgnet code, hopefully that will do.

If there is anything I can help with feel free to mention me here, I'll do my best to help.

Cheers!

[bburns632]

Hi @zeehio, thanks for the comment and support. Your change exposed a logical gap in the recursion implemented in pkgnet. PR #322 will resolve. There is no issue in R6.

Thanks!

[bburns632]

To clarify, these errors were caused by pkgnet's lacking support for control statements like break and next. When the function parsers, .parse_function or .parse_R6_expression would encounter such a statement, they would return an empty list that would break the recursion.

[jayqi]

Closed by #322