Merge commit '597bb04a708ccc2f07d420823bf998fc7f037355'

Author: hadley

.claude/settings.local.json

@@ -5,7 +5,10 @@
       "Bash(find:*)",
       "Bash(R:*)",
       "Bash(rm:*)",
-      "Bash(air format:*)"
+      "Bash(air format:*)",
+      "Edit(R/**)",
+      "Edit(tests/**)",
+      "Edit(vignettes/**)"
     ],
     "deny": []
   }

.github/workflows/claude-code-review.yml

@@ -0,0 +1,78 @@
+name: Claude Code Review
+
+on:
+  pull_request:
+    types: [opened, synchronize]
+    # Optional: Only run on specific file changes
+    # paths:
+    #   - "src/**/*.ts"
+    #   - "src/**/*.tsx"
+    #   - "src/**/*.js"
+    #   - "src/**/*.jsx"
+
+jobs:
+  claude-review:
+    # Optional: Filter by PR author
+    # if: |
+    #   github.event.pull_request.user.login == 'external-contributor' ||
+    #   github.event.pull_request.user.login == 'new-developer' ||
+    #   github.event.pull_request.author_association == 'FIRST_TIME_CONTRIBUTOR'
+    
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      pull-requests: read
+      issues: read
+      id-token: write
+    
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 1
+
+      - name: Run Claude Code Review
+        id: claude-review
+        uses: anthropics/claude-code-action@beta
+        with:
+          anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
+
+          # Optional: Specify model (defaults to Claude Sonnet 4, uncomment for Claude Opus 4.1)
+          # model: "claude-opus-4-1-20250805"
+
+          # Direct prompt for automated review (no @claude mention needed)
+          direct_prompt: |
+            Please review this pull request and provide feedback on:
+            - Code quality and best practices
+            - Potential bugs or issues
+            - Performance considerations
+            - Security concerns
+            - Test coverage
+            
+            Be constructive and helpful in your feedback.
+
+          # Optional: Use sticky comments to make Claude reuse the same comment on subsequent pushes to the same PR
+          # use_sticky_comment: true
+          
+          # Optional: Customize review based on file types
+          # direct_prompt: |
+          #   Review this PR focusing on:
+          #   - For TypeScript files: Type safety and proper interface usage
+          #   - For API endpoints: Security, input validation, and error handling
+          #   - For React components: Performance, accessibility, and best practices
+          #   - For tests: Coverage, edge cases, and test quality
+          
+          # Optional: Different prompts for different authors
+          # direct_prompt: |
+          #   ${{ github.event.pull_request.author_association == 'FIRST_TIME_CONTRIBUTOR' && 
+          #   'Welcome! Please review this PR from a first-time contributor. Be encouraging and provide detailed explanations for any suggestions.' ||
+          #   'Please provide a thorough code review focusing on our coding standards and best practices.' }}
+          
+          # Optional: Add specific tools for running tests or linting
+          # allowed_tools: "Bash(npm run test),Bash(npm run lint),Bash(npm run typecheck)"
+          
+          # Optional: Skip review for certain conditions
+          # if: |
+          #   !contains(github.event.pull_request.title, '[skip-review]') &&
+          #   !contains(github.event.pull_request.title, '[WIP]')
+

.github/workflows/claude.yml

@@ -0,0 +1,75 @@
+name: Claude Code
+
+on:
+  issue_comment:
+    types: [created]
+  pull_request_review_comment:
+    types: [created]
+  issues:
+    types: [opened, assigned]
+  pull_request_review:
+    types: [submitted]
+
+jobs:
+  claude:
+    if: |
+      (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) ||
+      (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) ||
+      (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) ||
+      (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude')))
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      pull-requests: read
+      issues: read
+      id-token: write
+      actions: read # Required for Claude to read CI results on PRs
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 1
+
+      - name: Install air
+        uses: posit-dev/setup-air@v1
+
+      - uses: r-lib/actions/setup-r@v2
+        with:
+          use-public-rspm: true
+
+      - uses: r-lib/actions/setup-r-dependencies@v2
+        with:
+          extra-packages: any::rcmdcheck,roxygen2
+          needs: check
+
+      - name: Run Claude Code
+        id: claude
+        uses: anthropics/claude-code-action@beta
+        with:
+          anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
+
+          # This is an optional setting that allows Claude to read CI results on PRs
+          additional_permissions: |
+            actions: read
+          
+          # Optional: Specify model (defaults to Claude Sonnet 4, uncomment for Claude Opus 4.1)
+          # model: "claude-opus-4-1-20250805"
+          
+          # Optional: Customize the trigger phrase (default: @claude)
+          # trigger_phrase: "/claude"
+          
+          # Optional: Trigger when specific user is assigned to an issue
+          # assignee_trigger: "claude-bot"
+          
+          # Optional: Allow Claude to run specific commands
+          allowed_tools: "Bash(R:*);Bash(air format:*)"
+          
+          # Optional: Add custom instructions for Claude to customize its behavior for your project
+          # custom_instructions: |
+          #   Follow our coding standards
+          #   Ensure all new code has tests
+          #   Use TypeScript for new files
+          
+          # Optional: Custom environment variables for Claude
+          # claude_env: |
+          #   NODE_ENV: test

NEWS.md

@@ -1,5 +1,8 @@
 # testthat (development version)
 
+* `expect_snapshot_file(name=)` must have a unique file path. If a snapshot file attempts to be saved with a duplicate `name`, an error will be thrown. (#1592)
+* `test_dir()`, `test_file()`, `test_package()`, `test_check()`, `test_local()`, `source_file()` gain a `shuffle` argument uses `sample()` to randomly reorder the top-level expressions in each test file (#1942). This random reordering surfaces dependencies between tests and code outside of any test, as well as dependencies between tests. This helps you find and eliminate unintentional dependencies.
+* `snapshot_accept(test)` now works when the test file name contains `.` (#1669).
 * `local_mock()` and `with_mock()` have been deprecated because they are no longer permitted in R 4.5.
 * `snapshot_review()` now passes `...` on to `shiny::runApp()` (#1928).
 * `expect_named()` now gives more informative errors (#2091).
@@ -41,7 +44,7 @@
 * Fixed an issue preventing compilation from succeeding due to deprecation / removal of `std::uncaught_exception()` (@kevinushey, #2047).
 * New `skip_unless_r()` to skip running tests on unsuitable versions of R, e.g. `skip_unless_r(">= 4.1.0")` to skip tests that require, say, `...names` (@MichaelChirico, #2022)
 * `skip_on_os()` gains an option `"emscripten"` of the `os` argument to skip tests on Emscripten (@eitsupi, #2103).
-* New expectation, `expect_shape()`, for testing the shape (i.e., the `length()`, `nrow()`, `ncol()`, or `dim()`), all in one place (#1423, @michaelchirico).
+* New expectation, `expect_shape()`, for testing the shape (i.e., the `nrow()`, `ncol()`, or `dim()`), all in one place (#1423, @michaelchirico).
 
 # testthat 3.2.3
 

R/expect-length.R

@@ -1,21 +1,44 @@
-#' Do you expect an object with this shape?
+#' Do you expect an object with this length or shape?
 #'
-#' This is a generalization of [expect_length()] to test the "shape" of
-#' more general objects like data.frames, matrices, and arrays.
+#' `expect_length()` inspects the [length()] of an object; `expect_shape()`
+#' inspects the "shape" (i.e. [nrow()], [ncol()], or [dim()]) of
+#' higher-dimensional objects like data.frames, matrices, and arrays.
 #'
-#' @seealso [expect_length()] to specifically make assertions about the
-#'   [length()] of a vector.
+#' @seealso [expect_vector()] to make assertions about the "size" of a vector.
 #' @inheritParams expect_that
-#' @param ... Ignored.
-#' @param nrow,ncol Expected [nrow()]/[ncol()] of `object`.
-#' @param dim Expected [dim()] of `object`.
+#' @param n Expected length.
 #' @family expectations
 #' @export
 #' @examples
+#' expect_length(1, 1)
+#' expect_length(1:10, 10)
+#' show_failure(expect_length(1:10, 1))
+#'
 #' x <- matrix(1:9, nrow = 3)
 #' expect_shape(x, nrow = 3)
+#' show_failure(expect_shape(x, nrow = 4))
 #' expect_shape(x, ncol = 3)
+#' show_failure(expect_shape(x, ncol = 4))
 #' expect_shape(x, dim = c(3, 3))
+#' show_failure(expect_shape(x, dim = c(3, 4, 5)))
+expect_length <- function(object, n) {
+  check_number_whole(n, min = 0)
+
+  act <- quasi_label(enquo(object))
+  act$n <- length(act$val)
+
+  if (act$n != n) {
+    msg <- sprintf("%s has length %i, not length %i.", act$lab, act$n, n)
+    return(fail(msg))
+  }
+  pass(act$val)
+}
+
+#' @param nrow,ncol Expected [nrow()]/[ncol()] of `object`.
+#' @param dim Expected [dim()] of `object`.
+#' @rdname expect_length
+#' @param ... Not used; used to force naming of other arguments.
+#' @export
 expect_shape = function(object, ..., nrow, ncol, dim) {
   check_dots_empty()
   check_exclusive(nrow, ncol, dim)

R/expect-shape.R

@@ -41,7 +41,8 @@ test_files_parallel <- function(
   stop_on_failure = FALSE,
   stop_on_warning = FALSE,
   wrap = TRUE, # unused, to match test_files signature
-  load_package = c("none", "installed", "source")
+  load_package = c("none", "installed", "source"),
+  shuffle = FALSE
 ) {
   # TODO: support timeouts. 20-30s for each file by default?
 
@@ -59,7 +60,8 @@ test_files_parallel <- function(
     test_dir = test_dir,
     load_helpers = load_helpers,
     num_workers = num_workers,
-    load_package = load_package
+    load_package = load_package,
+    shuffle = shuffle
   )
 
   withr::with_dir(test_dir, {
@@ -210,7 +212,8 @@ queue_setup <- function(
   test_dir,
   num_workers,
   load_helpers,
-  load_package
+  load_package,
+  shuffle = FALSE
 ) {
   # TODO: observe `load_package`, but the "none" default is not
   # OK for the subprocess, because it'll not have the tested package
@@ -249,9 +252,11 @@ queue_setup <- function(
   })
   queue <- task_q$new(concurrency = num_workers, load_hook = load_hook)
 
-  fun <- transport_fun(function(path) asNamespace("testthat")$queue_task(path))
+  fun <- transport_fun(function(path, shuffle) {
+    asNamespace("testthat")$queue_task(path, shuffle)
+  })
   for (path in test_paths) {
-    queue$push(fun, list(path))
+    queue$push(fun, list(path, shuffle))
   }
 
   queue
@@ -282,19 +287,19 @@ queue_process_setup <- function(
   the$testing_env <- env
 }
 
-queue_task <- function(path) {
+queue_task <- function(path, shuffle = FALSE) {
   withr::local_envvar("TESTTHAT_IS_PARALLEL" = "true")
-  snapshotter <- SubprocessSnapshotReporter$new(
-    snap_dir = "_snaps",
-    fail_on_new = FALSE
-  )
+  snapshotter <- SubprocessSnapshotReporter$new(snap_dir = "_snaps")
   withr::local_options(testthat.snapshotter = snapshotter)
   reporters <- list(
     SubprocessReporter$new(),
     snapshotter
   )
   multi <- MultiReporter$new(reporters = reporters)
-  with_reporter(multi, test_one_file(path, env = the$testing_env))
+  with_reporter(
+    multi,
+    test_one_file(path, env = the$testing_env, shuffle = shuffle)
+  )
   NULL
 }
 

R/parallel.R

@@ -137,8 +137,11 @@ expect_snapshot_file <- function(
     path,
     file_equal = compare,
     variant = variant,
-    trace_env = caller_env()
   )
+  if (inherits(equal, "expectation_failure")) {
+    return(equal)
+  }
+
   hint <- snapshot_review_hint(snapshotter$file, name)
 
   if (!equal) {
@@ -196,18 +199,19 @@ snapshot_file_equal <- function(
   snap_variant, # variant (optional)
   path, # path to new file
   file_equal = compare_file_binary,
-  fail_on_new = FALSE,
+  fail_on_new = NULL,
   trace_env = caller_env()
 ) {
   if (!file.exists(path)) {
-    cli::cli_abort("{.path {path}} not found.")
+    cli::cli_abort("{.path {path}} not found.", call = trace_env)
   }
 
   if (is.null(snap_variant)) {
     snap_test_dir <- file.path(snap_dir, snap_test)
   } else {
     snap_test_dir <- file.path(snap_dir, snap_variant, snap_test)
   }
+  fail_on_new <- fail_on_new %||% on_ci()
 
   cur_path <- file.path(snap_test_dir, snap_name)
   new_path <- file.path(snap_test_dir, new_name(snap_name))
@@ -232,12 +236,13 @@ snapshot_file_equal <- function(
       snap_name,
       "'"
     )
+
+    # We want to fail on CI since this suggests that the user has failed
+    # to record the value locally
     if (fail_on_new) {
       return(fail(message, trace_env = trace_env))
-    } else {
-      testthat_warn(message)
     }
-
+    testthat_warn(message)
     TRUE
   }
 }

R/snapshot-file.R

@@ -196,7 +196,8 @@ snapshot_meta <- function(files = NULL, path = "tests/testthat") {
     files <- files[!is_dir]
 
     dirs <- substr(dirs, 1, nchar(dirs) - 1)
-    files <- ifelse(tools::file_ext(files) == "", paste0(files, ".md"), files)
+    # Match regardless of whether user include .md or not
+    files <- c(files, paste0(files, ".md"))
 
     out <- out[out$name %in% files | out$test %in% dirs, , drop = FALSE]
   }