Skip to content

Conversation

@maelle
Copy link
Contributor

@maelle maelle commented Feb 21, 2025

@maelle
Copy link
Contributor Author

maelle commented Feb 21, 2025

The failure on Ubuntu seems unrelated to the changes

══ Failed tests ════════════════════════════════════════════════════════════════
── Error ('test-local.R:38:3'): can override translation of error messages ─────
<notSubsettableError/error/condition>
Error in `mean[[1]]`: object of type 'closure' is not subsettable

i = "You can set {.arg recycle} to {.code TRUE}."
))
}
value <- rep_len(values, length.out = i)[[i]]
Copy link
Member

Choose a reason for hiding this comment

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

It'd be a little more efficient to do this with modular arithmetic, something like i <- (i - 1) %% length(values) + 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aaaah I had not found this elegant solution. Code now refactored!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(I had been using the modulo operator with if and else... 🤪 )

#' recycled_mocked_sequence()
#' recycled_mocked_sequence()
#' @family mocking
mock_output_sequence <- function(values, recycle = FALSE) {
Copy link
Member

Choose a reason for hiding this comment

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

Should we use ... instead of values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be sure I understand: do you mean that instead of writing mock_output_sequence(c("blop", "bla")) the user would write mock_output_sequence("blop", "bla")?

And using rlang's "Dynamic dots features"?

Copy link
Member

@hadley hadley Feb 25, 2025

Choose a reason for hiding this comment

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

Yes and yes 😄

expect_snapshot(mocked_sequence(), error = TRUE)
})

test_that("mock_output_sequence() works -- force", {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to write a test that'd fail if I didn't use force() but it doesn't fail so I removed force(). Maybe because of list2()? Or maybe because I missed something obvious. 😅

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it's no longer needed because list2 forces all the values in ...

Copy link
Member

Choose a reason for hiding this comment

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

So you can remove this test IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, done

@hadley hadley merged commit 30f5b11 into r-lib:main Feb 28, 2025
4 of 11 checks passed
@hadley
Copy link
Member

hadley commented Feb 28, 2025

Thanks!

schloerke added a commit to schloerke/testthat that referenced this pull request Jul 25, 2025
* upstream/HEAD: (250 commits)
  Implement `expect_shape()` (r-lib#1469)
  Re-enable catch tests on windows (r-lib#2104)
  Move digest to suggests (r-lib#2105)
  Support emscripten in `skip_on_os()` (r-lib#2103)
  Add `skip_unless_r()` (r-lib#2094)
  Require R 4.1 (r-lib#2101)
  Allow unquoting first arg for `expect_s4_class()` (r-lib#2065)
  typo fix (r-lib#2051)
  Explicitly pass `parent.frame()` in `it()` (r-lib#2086)
  Update vignette advice for migrating to {testthat} 3e (r-lib#2080)
  New praise messages (r-lib#1974)
  move '!' outside aggregation (r-lib#2067)
  Handle deprecation of std::uncaught_exception() (r-lib#2097)
  perf: use `anyNA()` (r-lib#2058)
  Upkeep (r-lib#2099)
  Implement `mock_output_sequence()` (r-lib#2061)
  Increment version number to 3.2.3.9000
  Increment version number to 3.2.3
  Update CRAN comments
  Finish off r-lib#2046
  ...
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.

2 participants