Skip to content

Add tests for translations in translations.R #28

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
StevenHibble opened this issue Feb 23, 2020 · 3 comments
Open

Add tests for translations in translations.R #28

StevenHibble opened this issue Feb 23, 2020 · 3 comments

Comments

@StevenHibble
Copy link
Contributor

There are data types, operators, functions, and aggregates that need testing. The thought process was laid out by Ian in this comment. How should we handle testing "generic" translations (where tidy = FALSE and tidy = TRUE return the same results)? Here's my current idea:

test_that("string is translated as character", {
  expect_equal(parse_expression("cast(a as string)", tidy = FALSE), str2lang("as.character(a)"))
  expect_equal(parse_expression("cast(a as string)", tidy = TRUE),  str2lang("as.character(a)"))
})

Maybe extracting the expressions would be better:

test_that("string is translated as character", {
  SQL <- "cast(a as string)"
  R   <- "as.character(a)"
  expect_equal(parse_expression(SQL, tidy = FALSE), str2lang(R))
  expect_equal(parse_expression(SQL, tidy = TRUE),  str2lang(R))
})
@StevenHibble
Copy link
Contributor Author

I'm thinking there are two options for "indirect" translations:

  1. Test the translation
test_that("ifnull() function translates correctly", {
  SQL <- "ifnull(a, b)"
  R   <- "ifelse(is.na(a), b, a)"
  expect_equal(parse_expression(SQL, tidy = FALSE), str2lang(R))
  expect_equal(parse_expression(SQL, tidy = TRUE),  str2lang(R))
})
  1. Test the outcome
test_that("ifnull() function translates correctly", {
  a <- c(1, 2, NA)
  b <- c(7, 8, 9)
  SQL <- "ifnull(a, b)"
  answer <- c(1, 2, 9)

  untidy <- eval(parse_expression(SQL, tidy = FALSE))
  tidy   <- eval(parse_expression(SQL, tidy = TRUE))
  expect_equal(untidy, answer)
  expect_equal(tidy,   answer)
})

I think I prefer the 2nd option, though it is more involved. Thoughts?

@ianmcook
Copy link
Owner

Thanks for putting some thought into this.

To reduce the repetition of the same boilerplate code in every test, we could define some helper functions. For example, we could have a trio of functions test_parse_expression_tidyverse(), test_parse_expression_base(), and test_parse_expression_generic(). The generic one would simply call both of the other two.

@ianmcook
Copy link
Owner

Regarding whether to (1) test the translation or (2) test the outcome:

I think (1) is safer and more general. It eliminates the possibility that the particular input values used in the test coincidentally worked but will not work in general.

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

No branches or pull requests

2 participants