From e6ad96f7b2a18738bd074b892206f12ae2023f9a Mon Sep 17 00:00:00 2001
From: Daniel Chen <chendaniely@users.noreply.github.com>
Date: Mon, 15 Jul 2019 11:34:31 -0700
Subject: [PATCH] Test and Fix for comparing long `quote` expressions (#34)

* tdd for bugfix

* find the other user_code == solution_code "bug"

* use more generic test case w/o packages

* user rlang::sim() instead of quote
---
 .Rbuildignore                    |  1 +
 .gitignore                       |  2 ++
 R/check_code.R                   |  2 +-
 R/detect_mistakes.R              |  3 ++-
 tests/testthat/test_check_code.R | 23 +++++++++++++++++++++++
 5 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/.Rbuildignore b/.Rbuildignore
index d6883bbd..73551ef7 100644
--- a/.Rbuildignore
+++ b/.Rbuildignore
@@ -8,3 +8,4 @@
 ^codecov\.yml$
 ^man-roxygen$
 ^\.lintr$
+^\.vscode$
diff --git a/.gitignore b/.gitignore
index e8fa31d2..6b198867 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,5 @@
+.vscode/
+
 # History files
 .Rhistory
 .Rapp.history
diff --git a/R/check_code.R b/R/check_code.R
index e4fe725a..b7b01588 100644
--- a/R/check_code.R
+++ b/R/check_code.R
@@ -125,7 +125,7 @@ code_is_same <- function(user = NULL, solution = NULL) {
   solution_code <- rlang::get_expr(solution)
 
   # Correct answers are all alike
-  if (suppressWarnings(user_code == solution_code)) {
+  if (identical(user_code, solution_code)) {
     return(graded(correct = TRUE, message = NULL))
   }
 
diff --git a/R/detect_mistakes.R b/R/detect_mistakes.R
index b2388c72..715d812f 100644
--- a/R/detect_mistakes.R
+++ b/R/detect_mistakes.R
@@ -31,7 +31,8 @@ detect_mistakes <- function(user,
 
     # Does the user code not match the solution code?
     if (length(user[[i]]) != length(solution[[i]]) ||
-        user[[i]] != solution[[i]]) {
+        !identical(user[[i]], solution[[i]])
+        ) {
       return(isolate_mismatch(user, solution, i))
     }
   }
diff --git a/tests/testthat/test_check_code.R b/tests/testthat/test_check_code.R
index 5c398d1a..78b25d40 100644
--- a/tests/testthat/test_check_code.R
+++ b/tests/testthat/test_check_code.R
@@ -5,6 +5,12 @@ expect_correct <- function(x) {
   expect_s3_class(x, "grader_graded")
   expect_true(x$correct)
 }
+
+expect_wrong <- function(x) {
+  expect_s3_class(x, "grader_graded")
+  expect_false(x$correct)
+}
+
 expect_message <- function(x, message) {
   expect_s3_class(x, "grader_graded")
   expect_true(!x$correct)
@@ -197,3 +203,20 @@ test_that("Spot differences when pipes are involved", {
   expect_correct(check_code(grader_args = list(user_quo = pipe3, solution_quo = pipe3)))
 
 })
+
+test_that("Spots differences in long calls", {
+  # original discussion here:
+  # https://github.com/rstudio-education/grader/issues/28
+
+  user <- rlang::sym("tidyr::gather(key = key, value = value, new_sp_m014:newrel_f65, na.rm = TRUE)") # nolint
+  solution <- rlang::sym("tidyr::gather(key = key, value = value, new_sp_m014:newrel_f65, na.rm = FALSE)") # nolint
+  expect_wrong(
+    check_code(grader_args = list(user_quo = user, solution_quo = solution))
+  )
+
+  user <- rlang::sym("tidyr::gather(key = key, value = value, new_sp_m014:newrel_f65, na.rm = TRUE)") # nolint
+  solution <- rlang::sym("tidyr::gather(key = key, value = value, new_sp_m014:newrel_f65, na.rm = TRUE)") # nolint
+   expect_correct(
+    check_code(grader_args = list(user_quo = user, solution_quo = solution))
+  )
+})