Skip to content
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

Move duplicate functions #145

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Conversation

yangsophieee
Copy link
Collaborator

@yangsophieee yangsophieee commented Dec 7, 2023

  • Fixes [traits.build tests] Duplicated tests between dataset_test and tests #131
  • Move custom expect_ functions out of dataset_test, into R/test_functions.R so that R CMD check warnings are removed (otherwise "no visible global function definition" warning comes up)
  • Rename prefixes to test_expect_ to differentiate between custom testthat functions and built-in testthat functions
  • I kept using expect_silent() from {testthat} instead of writing a custom function because I'm not sure how to rewrite the function to capture messages on top of errors and warnings

- Move custom `expect_` functions out of `dataset_test`, into R/test_functions.R so that R CMD check warnings are removed
- Rename prefixes to `test_`
tests/testthat/test-setup.R Show resolved Hide resolved
tests/testthat/test-setup.R Show resolved Hide resolved
tests/testthat/test-xamples.R Show resolved Hide resolved
tests/testthat/test-xamples.R Show resolved Hide resolved
R/test_functions.R Outdated Show resolved Hide resolved

# Expected output
tables <- c("traits", "locations", "contexts", "methods", "excluded_data",
"taxonomic_updates", "taxa", "contributors")
expect_no_error(
expected_output <-
purrr::map(
tables, ~read_csv(sprintf("examples/Test_2023_8/output/%s.csv", .x), col_types = cols(.default = "c"))),
info = "Reading in expected output tables"
Copy link
Member

Choose a reason for hiding this comment

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

Why delete? The info isn't helpful?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's using expect_no_error from {testthat} which does not accept an info argument.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, but you also had a new function defined called expect_no_error, which would overwrite the one from {test that}. Now it's renamed to test_ expect_no_error, so perhaps isn't being used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh there shouldn't have been a new function defined called expect_no_error; if there was- that's my bad!

I can't seem to find it though. I did a find and replace on test_ to test_expect_ before so theoretically I shouldn't have touched expect_no_error.

tests/testthat/test-xamples.R Show resolved Hide resolved
@yangsophieee
Copy link
Collaborator Author

@dfalster I should have explained clearer but basically R/test_functions.R contains 'custom' expect_ functions, now named test_expect_ functions, which allow you to customise the failure message. The inbuilt expect_ functions from {testthat} do not allow full customisation of failure messages. I have used the test_expect_ functions only in dataset_test(), and the rest of the tests use {testthat} inbuilt functions. Hope that makes sense!

@dfalster
Copy link
Member

dfalster commented Dec 8, 2023

@dfalster I should have explained clearer but basically R/test_functions.R contains 'custom' expect_ functions, now named test_expect_ functions, which allow you to customise the failure message. The inbuilt expect_ functions from {testthat} do not allow full customisation of failure messages. I have used the test_expect_ functions only in dataset_test(), and the rest of the tests use {testthat} inbuilt functions. Hope that makes sense!

Ok!

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.

[traits.build tests] Duplicated tests between dataset_test and tests
2 participants