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

Add a name parameter to allow for different fixtures #178

Open
Kjir opened this issue Jun 6, 2023 · 1 comment
Open

Add a name parameter to allow for different fixtures #178

Kjir opened this issue Jun 6, 2023 · 1 comment

Comments

@Kjir
Copy link

Kjir commented Jun 6, 2023

It would be quite handy to be able to test the behaviour of the code with different results from the query: what if the table is empty? What if the table has more (or less) rows than the pagination limit?

I would expect to be able to provide a name to the asset, so that the same query can be registered with different outcomes. This would probably indirectly solve #159 as well.

Example code:

start_db_capturing("no-airlines")

con <- DBI::dbConnect(
  RPostgres::Postgres(),
  dbname = "nycflights"
)

get_an_airline(con)
DBI::dbDisconnect(con)

stop_db_capturing()

And test:

with_mock_db({
  con <- DBI::dbConnect(
    RPostgres::Postgres(),
    dbname = "nycflights"
  )
  
  test_that("We get no airline", {
    one_airline <- get_an_airline(con)
    expect_s3_class(one_airline, "data.frame")
    expect_equal(nrow(one_airline), 0)
  })
}, case = "no-airlines")

This would allow to use dittodb to run integration tests as well, instead of being able to test only the DB-to-R portion of the code.

In theory this could be achieved using with_mock_path, but it does feel clunkier

@jonkeane
Copy link
Collaborator

jonkeane commented Aug 3, 2023

Thanks for the issue + the great reprex.

I think this is actually quite close to being a reality. start_db_capturing() already takes a path that it will save to, so that should be good. We could then add the case argument1 to with_mock_db() that then uses with_mock_path() internally when it is called.

Might you be interested in sending a PR that does that?

Footnotes

  1. At the risk of bikeshedding: I totally see where case comes from, but I wonder if it might be better to call this path to match that use elsewhere in the package?

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