Skip to content

Commit

Permalink
[188559300]: Update tests for new project behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
gergness committed Nov 18, 2024
1 parent 54171d4 commit 2aa6665
Show file tree
Hide file tree
Showing 14 changed files with 88 additions and 26 deletions.
5 changes: 3 additions & 2 deletions R/get-datasets.R
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ loadDataset <- function(dataset,
if (missing(project)) {
warn_once(
"Finding datasets by name without specifying a path is no longer supported.",
option = "find_dataset_no_project"
option = "find.dataset.no.project"
)
}
halt(dQuote(dataset), " not found")
Expand Down Expand Up @@ -243,6 +243,7 @@ lookupDataset <- function(x, project = NULL) {
# `project`
dspath <- parseFolderPath(x)
x <- tail(dspath, 1)

if (length(dspath) == 1 && is.null(project)) {
# This code path used to use the datasets by_name endpoint. However
# As of 2024-11, that endpoint is no longer very useful because it only
Expand All @@ -252,7 +253,7 @@ lookupDataset <- function(x, project = NULL) {
# To get here, a user had to explicitly set `project=NULL` so they're
# presumably not here accidentally
pseudo_shoji <- tryCatch({
ds_base_url <- absoluteURL(paste0("datasets/", x, "/"), envOrOption("crunch.api"))
ds_base_url <- absoluteURL("datasets/", envOrOption("crunch.api"))
ds_entity <- crGET(paste0(ds_base_url, "/", x))
# Need to make this pseudo DatasetCatalog to match old API call (because `loadDataset`
# will later pass it through `active()`/`archived()`)
Expand Down
12 changes: 11 additions & 1 deletion inst/crunch-test.R
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,18 @@ cachedLoadDataset <- function(dataset, ...) {
stop("non-dataset arguments ignored in cached datasets")
}
if (!dataset %in% names(ds_cache_env)) {
# replicate old dataset by name behavior
ds_id <- switch(
dataset,
"test ds" = "1",
"ECON.sav" = "3",
"test ds deck" = "4",
"Vegetables example" = "veg",
stop("Update cachedLoadDataset name->id crosswalk")
)

# Don't need `with_mock_crunch()` because caller is already inside it
ds_cache_env[[dataset]] <- loadDataset(dataset)
ds_cache_env[[dataset]] <- loadDataset(ds_id, project = NULL)
}
ds_cache_env[[dataset]]
}
Expand Down
3 changes: 2 additions & 1 deletion mocks/app.crunch.io/api/datasets/2.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
"body": {
"owner_id": "https://app.crunch.io/api/users/user1/",
"id": "aa80d2e969b34e6ea9ededaa528c6248",
"streaming": "no"
"streaming": "no",
"archived": true
},
"catalogs": {
"variables": "https://app.crunch.io/api/datasets/2/variables/",
Expand Down
4 changes: 3 additions & 1 deletion mocks/app.crunch.io/api/datasets/3.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
"id": "95c0b45fe0af492594863f818cb913d2",
"name": "ECON.sav",
"description": "",
"streaming": "no"
"streaming": "no",
"archived": false,
"is_published": false
},
"catalogs": {
"variables": "https://app.crunch.io/api/datasets/3/variables/",
Expand Down
43 changes: 43 additions & 0 deletions mocks/app.crunch.io/api/projects/project_dup.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
{
"element": "shoji:entity",
"self": "https://app.crunch.io/api/projects/project_dup/",
"catalogs": {
"datasets": "https://app.crunch.io/api/projects/project_dup/datasets/",
"users": "https://app.crunch.io/api/projects/project_dup/users/",
"members": "https://app.crunch.io/api/projects/project_dup/members/",
"project": "https://app.crunch.io/api/projects/"
},
"description": "Details for a specific project",
"body": {
"name": "Project with dups",
"description": "A project made withduplicated dataset names"
},
"index": {
"https://app.crunch.io/api/datasets/ac6248/": {
"owner_display_name": "Fake User",
"name": "duplicated dataset",
"description": "This dataset does not exist",
"archived": false,
"id": "ac6248",
"start_date": null,
"end_date": null,
"owner_id": "https://app.crunch.io/api/users/user1/",
"type": "dataset"
},
"https://app.crunch.io/api/datasets/6ea9ed/": {
"owner_display_name": "Fake User",
"name": "duplicated dataset",
"description": "This dataset also does not exist",
"archived": false,
"id": "6ea9ed",
"start_date": null,
"end_date": null,
"owner_id": "https://app.crunch.io/api/users/user1/",
"type": "dataset"
}
},
"graph": [
"https://app.crunch.io/api/datasets/ac6248/",
"https://app.crunch.io/api/datasets/6ea9ed/"
]
}
2 changes: 1 addition & 1 deletion tests/testthat/test-categories.R
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ with_mock_crunch({
})


ds_catdate <- loadDataset("cat date test")
ds_catdate <- loadDataset("b9d811", project = NULL)
cats <- categories(ds_catdate$cat_set)

test_that("catdates getters/setters", {
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-compare.R
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ test_that("print compareCategory summary when categories are equivalent", {

with_mock_crunch({
ds1 <- cachedLoadDataset("test ds")
ds2 <- loadDataset("an archived dataset", "archived")
ds2 <- loadDataset("2", "archived", project = NULL)

test_that("compareVariables", {
expect_equal(
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-cut.R
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ with_mock_crunch({
})

test_that("cut date returns expected output", {
ds_catdate <- loadDataset("cat date test")
ds_catdate <- loadDataset("b9d811", project = NULL)

simple_date_cut <- structure(list(
derivation = list(
Expand Down
9 changes: 5 additions & 4 deletions tests/testthat/test-dataset-entity.R
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ if (tolower(Sys.info()[["sysname"]]) != "windows") {
with_mock_crunch({
ds <- cachedLoadDataset("test ds")
ds2 <- cachedLoadDataset("ECON.sav")
ds3 <- loadDataset("an archived dataset", kind = "archived")
ds3 <- loadDataset("2", "archived", project = NULL)

today <- "2016-02-11"

Expand All @@ -15,7 +15,7 @@ if (tolower(Sys.info()[["sysname"]]) != "windows") {

test_that(
"Dataset can be loaded without cache (tests otherwise unused lazy-var-cat code path)", {
httpcache::uncached(ds <- loadDataset("test ds"))
httpcache::uncached(ds <- loadDataset("1", project = NULL)) # test ds
expect_true(is.dataset(ds))
})

Expand Down Expand Up @@ -486,7 +486,7 @@ if (tolower(Sys.info()[["sysname"]]) != "windows") {

with_consent({
test_that("deleteDataset by name", {
expect_DELETE(deleteDataset("test ds"), self(ds))
expect_DELETE(deleteDataset("test ds", project = "Project One"), self(ds))
})
test_that("deleteDataset by URL", {
expect_DELETE(deleteDataset(self(ds)), self(ds))
Expand All @@ -512,7 +512,8 @@ if (tolower(Sys.info()[["sysname"]]) != "windows") {
})
expect_error(deleteDataset(ds), "Must confirm")
expect_error(
deleteDataset("duplicated dataset"),
# I don't think this will be possible any more but it used to be in personal project?
deleteDataset("duplicated dataset", project = "http://app.crunch.io/api/projects/project_dup"),
paste(
dQuote("duplicated dataset"), "identifies 2 datasets.",
"To delete, please identify the dataset uniquely by URL or path."
Expand Down
6 changes: 3 additions & 3 deletions tests/testthat/test-dataset-stream.R
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ mock_stream_rows <- data.frame(
)

with_mock_crunch({
ds <- loadDataset("streaming test ds") ## has 2 messages waiting, 4 rows received
ds2 <- loadDataset("an archived dataset", kind = "archived") ## Has no streams mock
ds3 <- loadDataset("streaming no messages") ## has 0 messages waiting, 0 rows received
ds <- loadDataset("1streaming", project = NULL) ## has 2 messages waiting, 4 rows received
ds2 <- loadDataset("2", "archived", project = NULL) ## Has no streams mock
ds3 <- loadDataset("streaming-no-msg", project = NULL) ## has 0 messages waiting, 0 rows received
test_that("pendingStream gets pending messages", {
expect_equal(pendingStream(ds), 2)
expect_GET(pendingStream(ds2), "https://app.crunch.io/api/datasets/2/stream")
Expand Down
20 changes: 12 additions & 8 deletions tests/testthat/test-get-datasets.R
Original file line number Diff line number Diff line change
Expand Up @@ -73,13 +73,13 @@ with_mock_crunch({
})

test_that("loadDataset loads", {
ds <- loadDataset("test ds")
ds <- loadDataset("test ds", project = "Project One")
expect_true(is.dataset(ds))
expect_identical(name(ds), "test ds")
})

test_that("loadDataset by index is deprecated", {
expect_deprecated(ds <- loadDataset(1))
expect_deprecated(ds <- loadDataset(1, project = "Project One"))
expect_true(is.dataset(ds))
expect_identical(name(ds), "test ds")
expect_error(
Expand All @@ -100,7 +100,7 @@ with_mock_crunch({
})

test_that("loadDataset by URL when in main catalog", {
ds <- loadDataset("test ds")
ds <- loadDataset("test ds", project = "Project One")
ds_by_url <- loadDataset(self(ds))
expect_is(ds_by_url, "CrunchDataset")
expect_identical(name(ds), name(ds_by_url))
Expand Down Expand Up @@ -134,19 +134,23 @@ with_mock_crunch({
test_that("loadDataset(refresh=TRUE) drops caches", {
with(temp.option(httpcache.log = ""), {
logs <- capture.output({
loadDataset("test ds", refresh = TRUE)
loadDataset("test ds", refresh = TRUE, project = "Project One")
})
})
in_logs <- function(str, loglines) {
any(grepl(str, loglines, fixed = TRUE))
}
expect_true(in_logs("CACHE DROP ^https://app[.]crunch[.]io/api/datasets/by_name/", logs))
expect_true(in_logs("CACHE DROP ^https://app[.]crunch[.]io/api/projects/", logs))
})

test_that("loadDataset error handling", {
expect_error(
loadDataset("not a dataset"),
paste(dQuote("not a dataset"), "not found")
options(find.dataset.no.project = NULL)
expect_warning(
expect_error(
loadDataset("not a dataset"),
paste(dQuote("not a dataset"), "not found")
),
"Finding datasets by name without specifying a path is no longer supported"
)
expect_error(
loadDataset("not a dataset", project = 42),
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-private-variables.R
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
context("Private variables")

with_mock_crunch({
ds <- loadDataset("Private test")
ds <- loadDataset("5", project = NULL)
test_that("privateVariables", {
expect_identical(privateVariables(ds, "name"), "ID Number")
expect_identical(privateVariables(ds), "id")
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-update-with-missing.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ context("Update variables with NAs")

with_mock_crunch({
ds <- cachedLoadDataset("test ds")
ds2 <- loadDataset("an archived dataset", kind = "archived")
ds2 <- loadDataset("2", "archived", project = NULL)

test_that("If an array gets NA assigned and it doesn't have No Data, we add that category", {
expect_PATCH(
Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/test-variables.R
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ context("Variables")

with_mock_crunch({
ds <- cachedLoadDataset("test ds")
ds2 <- loadDataset("an archived dataset", "archived")
ds2 <- loadDataset("2", "archived", project = NULL)

test_that("Variable init, as, is", {
expect_true(is.variable(ds[[1]]))
Expand Down

0 comments on commit 2aa6665

Please sign in to comment.