diff --git a/R/add_resource.R b/R/add_resource.R index 7dc3937c..2b39bb21 100644 --- a/R/add_resource.R +++ b/R/add_resource.R @@ -81,28 +81,33 @@ add_resource <- function(package, resource_name, data, schema = NULL, check_package(package) # Check resource name - assertthat::assert_that( - grepl(resource_name, pattern = "^[a-z0-9\\._-]+$"), - msg = glue::glue( - "`resource_name` `{resource_name}` must only contain lowercase", - "alphanumeric characters plus `.`, `-` and `_`.", - .sep = " " + if (!grepl(resource_name, pattern = "^[a-z0-9\\._-]+$")) { + cli::cli_abort( + c( + "{.arg resource_name} must only consist of lowercase alphanumeric + characters, {.val .}, {.val -} and {.val _}.", + "x" = "{.val {resource_name}} does not meet those criteria." + ), + class = "frictionless_error_resource_name_invalid" ) - ) + } # Check resource is absent - assertthat::assert_that( - !resource_name %in% resources(package), - msg = glue::glue( - "`package` already contains a resource named `{resource_name}`." + if (resource_name %in% resources(package)) { + cli::cli_abort( + "{.arg package} already contains a resource named {.val {resource_name}}.", + class = "frictionless_error_resource_already_exists" + ) + } + + # Check data (data frame or path), content of data frame is checked later + if (!is.data.frame(data) && !is.character(data)) { + cli::cli_abort( + "{.arg data} must either be a data frame or path(s) to CSV file(s).", + class = "frictionless_error_data_type_invalid" ) - ) + } - # Check data (df or path) - assertthat::assert_that( - is.data.frame(data) | is.character(data), - msg = "`data` must be a data frame or path(s) to CSV file(s)." - ) if (is.data.frame(data)) { df <- data } else { @@ -129,22 +134,27 @@ add_resource <- function(package, resource_name, data, schema = NULL, check_schema(schema, df) # Check ellipsis - assertthat::assert_that( - ...length() == length(...names()), - msg = "All arguments in `...` must be named." - ) + if (...length() != length(...names())) { + cli::cli_abort( + "All arguments in {.arg ...} must be named.", + class = "frictionless_error_unnamed_argument" + ) + } properties <- ...names() reserved_properties <- c( "name", "path", "profile", "format", "mediatype", "encoding", "dialect" ) # data and schema are also reserved, but are named arguments conflicting_properties <- properties[properties %in% reserved_properties] - assertthat::assert_that( - length(conflicting_properties) == 0, - msg = glue::glue( - "`{conflicting_properties[1]}` must be removed as an argument. ", - "It is automatically added as a resource property by the function." + if (length(conflicting_properties) != 0) { + cli::cli_abort( + c( + "{.arg {conflicting_properties}} must be removed as argument{?s}.", + "i" = "{.field {conflicting_properties}} {?is/are} automatically added + as resource propert{?y/ies}." + ), + class = "frictionless_error_resource_properties_reserved" ) - ) + } # Create resource, with properties in specific order if (is.data.frame(data)) { diff --git a/R/check_schema.R b/R/check_schema.R index 9d4b57b3..702bba11 100644 --- a/R/check_schema.R +++ b/R/check_schema.R @@ -47,7 +47,7 @@ check_schema <- function(schema, data = NULL) { if (length(invalid_types) > 0) { cli::cli_abort( c( - "All fields in {.arg schema} must have a valid {.field type}.", + "All fields in {.arg schema} must have a valid {.field type} property.", "x" = "Type{?s} {.val {invalid_types}} {?is/are} invalid." ), class = "frictionless_error_fields_type_invalid" @@ -63,8 +63,8 @@ check_schema <- function(schema, data = NULL) { cli::cli_abort( c( "Field names in {.arg schema} must match column names in {.arg data}.", - "i" = "Field name{?s}: {.val {field_names}}", - "i" = "Column name{?s}: {.val {col_names}}" + "i" = "Field name{?s}: {.val {field_names}}.", + "i" = "Column name{?s}: {.val {col_names}}." ), class = "frictionless_error_fields_colnames_mismatch" ) diff --git a/R/create_schema.R b/R/create_schema.R index 2250a594..ef359204 100644 --- a/R/create_schema.R +++ b/R/create_schema.R @@ -72,7 +72,7 @@ create_schema <- function(data) { # Columns with all NA are considered logical by R (and read_delim) # Set those to character, since string is a better default for Table Schema data_as_list <- lapply(data, function(x) { - if (is.logical(x) & all(is.na(x))) { + if (is.logical(x) && all(is.na(x))) { as.character(x) } else { x @@ -120,7 +120,7 @@ create_schema <- function(data) { # Remove elements that are NULL or empty list schema <- clean_list( schema, - function(x) is.null(x) | length(x) == 0L, + function(x) is.null(x) || length(x) == 0L, recursive = TRUE ) diff --git a/R/get_resource.R b/R/get_resource.R index fa1626f5..1e1789ea 100644 --- a/R/get_resource.R +++ b/R/get_resource.R @@ -22,28 +22,28 @@ get_resource <- function(package, resource_name) { # Check resource resource_names <- resources(package) - assertthat::assert_that( - resource_name %in% resource_names, - msg = glue::glue( - "Can't find resource `{resource_name}` in {resource_names_collapse}.", - resource_names_collapse = glue::glue_collapse( - glue::backtick(resource_names), - sep = ", " - ) + if (!resource_name %in% resources(package)) { + cli::cli_abort( + c( + "Can't find resource {.val {resource_name}} in {.arg package}.", + "i" = "Available resource{?s}: {.val {resources(package)}}." + ), + class = "frictionless_error_resource_not_found" ) - ) + } # Get resource resource <- purrr::keep(package$resources, ~ .x$name == resource_name)[[1]] # Check path(s) to file(s) # https://specs.frictionlessdata.io/data-resource/#data-location - assertthat::assert_that( - !is.null(resource$path) | !is.null(resource$data), - msg = glue::glue( - "Resource `{resource_name}` must have property `path` or `data`." + if (is.null(resource$path) && is.null(resource$data)) { + cli::cli_abort( + "Resource {.val {resource_name}} must have a {.field path} or + {.field data} property.", + class = "frictionless_error_resource_without_path_data" ) - ) + } # Assign read_from property (based on path, then df, then data) if (length(resource$path) != 0) { diff --git a/R/read_package.R b/R/read_package.R index cf25e736..aea7b34e 100644 --- a/R/read_package.R +++ b/R/read_package.R @@ -34,7 +34,7 @@ read_package <- function(file = "datapackage.json") { # Check resources # https://specs.frictionlessdata.io/data-package/#metadata assertthat::assert_that( - length(descriptor$resources) != 0 & # Null or empty list + length(descriptor$resources) != 0 && # Null or empty list purrr::every(descriptor$resources, ~ !is.null(.x$name)), msg = glue::glue( "Descriptor `{file}` must have property `resources` containing at least", diff --git a/tests/testthat/test-add_resource.R b/tests/testthat/test-add_resource.R index e3449f77..085ded00 100644 --- a/tests/testthat/test-add_resource.R +++ b/tests/testthat/test-add_resource.R @@ -23,20 +23,47 @@ test_that("add_resource() returns error when resource name contains invalid characters", { p <- example_package df <- data.frame("col_1" = c(1, 2), "col_2" = c("a", "b")) + + # Invalid names + expect_error( + add_resource(p, "New", df), + class = "frictionless_error_resource_name_invalid" + ) expect_error( add_resource(p, "New", df), - paste( - "`New` must only contain lowercase alphanumeric characters plus", - "`.`, `-` and `_`." + regexp = paste( + "`resource_name` must only consist of lowercase alphanumeric characters,", + "\".\", \"-\" and \"_\"." ), fixed = TRUE ) - expect_error(add_resource(p, "nëw", df), "only contain lowercase") - expect_error(add_resource(p, " new", df), "only contain lowercase") - expect_error(add_resource(p, "new ", df), "only contain lowercase") - expect_error(add_resource(p, "n ew", df), "only contain lowercase") - expect_error(add_resource(p, "n/ew", df), "only contain lowercase") + expect_error( + add_resource(p, "New", df), + regexp = "\"New\" does not meet those criteria.", + fixed = TRUE + ) + expect_error( + add_resource(p, "nëw", df), + class = "frictionless_error_resource_name_invalid" + ) + expect_error( + add_resource(p, " new", df), + class = "frictionless_error_resource_name_invalid" + ) + expect_error( + add_resource(p, "new ", df), + class = "frictionless_error_resource_name_invalid" + ) + expect_error( + add_resource(p, "n ew", df), + class = "frictionless_error_resource_name_invalid" + ) + expect_error( + add_resource(p, "n/ew", df), + class = "frictionless_error_resource_name_invalid" + ) + # Valid names expect_true(check_package(add_resource(p, "n.ew", df))) expect_true(check_package(add_resource(p, "n-ew", df))) expect_true(check_package(add_resource(p, "n_ew", df))) @@ -50,7 +77,11 @@ test_that("add_resource() returns error when resource of that name already df <- data.frame("col_1" = c(1, 2), "col_2" = c("a", "b")) expect_error( add_resource(p, "deployments", df), - "`package` already contains a resource named `deployments`.", + class = "frictionless_error_resource_already_exists" + ) + expect_error( + add_resource(p, "deployments", df), + regexp = "`package` already contains a resource named \"deployments\".", fixed = TRUE ) }) @@ -60,7 +91,11 @@ test_that("add_resource() returns error when data is not data frame or p <- example_package expect_error( add_resource(p, "new", list()), - "`data` must be a data frame or path(s) to CSV file(s).", + class = "frictionless_error_data_type_invalid" + ) + expect_error( + add_resource(p, "new", list()), + regexp = "`data` must either be a data frame or path(s) to CSV file(s).", fixed = TRUE ) }) @@ -71,13 +106,11 @@ test_that("add_resource() returns error on invalid or empty data frame", { schema <- create_schema(df) expect_error( add_resource(p, "new", data.frame("col_1" = character(0))), - "`data` must be a data frame containing data.", - fixed = TRUE + class = "frictionless_error_data_invalid" ) expect_error( add_resource(p, "new", data.frame("col_1" = character(0)), schema), - "`data` must be a data frame containing data.", - fixed = TRUE + class = "frictionless_error_data_invalid" ) # For more tests see test-check_schema.R @@ -140,6 +173,10 @@ test_that("add_resource() returns error if ... arguments are unnamed", { p <- create_package() df <- data.frame("col_1" = c(1, 2), "col_2" = c("a", "b")) schema <- create_schema(df) + expect_error( + add_resource(p, "new", df, schema, delim = ",", "unnamed_value"), + class = "frictionless_error_unnamed_argument" + ) expect_error( add_resource(p, "new", df, schema, delim = ",", "unnamed_value"), "All arguments in `...` must be named.", @@ -152,18 +189,31 @@ test_that("add_resource() returns error if ... arguments are reserved", { df <- data.frame("col_1" = c(1, 2), "col_2" = c("a", "b")) expect_error( add_resource(p, "new", df, name = "custom_name"), - paste( - "`name` must be removed as an argument.", - "It is automatically added as a resource property by the function." - ), + class = "frictionless_error_resource_properties_reserved" + ) + expect_error( + add_resource(p, "new", df, name = "custom_name", format = "custom_format"), + class = "frictionless_error_resource_properties_reserved" + ) + + expect_error( + add_resource(p, "new", df, name = "custom_name"), + regexp = "`name` must be removed as argument.", fixed = TRUE ) expect_error( - add_resource(p, "new", df, path = "custom_path", encoding = "utf8"), - paste( - "`path` must be removed as an argument.", # First conflicting argument - "It is automatically added as a resource property by the function." - ), + add_resource(p, "new", df, name = "custom_name"), + regexp = "name is automatically added as resource property.", + fixed = TRUE + ) + expect_error( + add_resource(p, "new", df, name = "custom_name", format = "custom_format"), + regexp = "`name` and `format` must be removed as arguments.", + fixed = TRUE + ) + expect_error( + add_resource(p, "new", df, name = "custom_name", format = "custom_format"), + regexp = "name and format are automatically added as resource properties.", fixed = TRUE ) }) diff --git a/tests/testthat/test-check_schema.R b/tests/testthat/test-check_schema.R index ad594bfe..82bf9ee9 100644 --- a/tests/testthat/test-check_schema.R +++ b/tests/testthat/test-check_schema.R @@ -80,7 +80,7 @@ test_that("check_schema() returns error when Table Schema fields have invalid ty ) expect_error( check_schema(invalid_schema), - regexp = "All fields in `schema` must have a valid type.", + regexp = "All fields in `schema` must have a valid type property.", fixed = TRUE ) expect_error( @@ -149,12 +149,12 @@ test_that("check_schema() returns error on mismatching schema and data frame", { ) expect_error( check_schema(invalid_schema, df), - regexp = "Field names: \"col_2\" and \"col_1\"", + regexp = "Field names: \"col_2\" and \"col_1\".", fixed = TRUE ) expect_error( check_schema(invalid_schema, df), - regexp = "Column names: \"col_1\" and \"col_2\"", # Same for other tests + regexp = "Column names: \"col_1\" and \"col_2\".", # Same for other tests fixed = TRUE ) @@ -184,7 +184,7 @@ test_that("check_schema() returns error on mismatching schema and data frame", { ) expect_error( check_schema(invalid_schema, df), - regexp = "Field names: \"col_1\", \"col_2\", and \"col_3\"", + regexp = "Field names: \"col_1\", \"col_2\", and \"col_3\".", fixed = TRUE ) }) diff --git a/tests/testthat/test-read_resource.R b/tests/testthat/test-read_resource.R index 361855b8..8cfd8940 100644 --- a/tests/testthat/test-read_resource.R +++ b/tests/testthat/test-read_resource.R @@ -109,11 +109,7 @@ test_that("read_resource() returns error on invalid resource", { # No such resource expect_error( read_resource(p, "no_such_resource"), - paste( - "Can't find resource `no_such_resource` in `deployments`,", - "`observations`, `media`." - ), - fixed = TRUE + class = "frictionless_error_resource_not_found" ) # Create invalid package and add properties one by one to pass errors @@ -123,7 +119,11 @@ test_that("read_resource() returns error on invalid resource", { # No path or data expect_error( read_resource(p_invalid, "deployments"), - "Resource `deployments` must have property `path` or `data`.", + class = "frictionless_error_resource_without_path_data" + ) + expect_error( + read_resource(p_invalid, "deployments"), + regexp = "Resource \"deployments\" must have a path or data property.", fixed = TRUE ) diff --git a/tests/testthat/test-remove_resource.R b/tests/testthat/test-remove_resource.R index 38484d72..f2149494 100644 --- a/tests/testthat/test-remove_resource.R +++ b/tests/testthat/test-remove_resource.R @@ -16,10 +16,16 @@ test_that("remove_resource() returns error when resource not found", { p <- example_package expect_error( remove_resource(p, "no_such_resource"), - paste( - "Can't find resource `no_such_resource` in `deployments`,", - "`observations`, `media`." - ), + class = "frictionless_error_resource_not_found" + ) + expect_error( + remove_resource(p, "no_such_resource"), + regexp = "Can't find resource \"no_such_resource\" in `package`", + fixed = TRUE + ) + expect_error( + remove_resource(p, "no_such_resource"), + regexp = "Available resources: \"deployments\", \"observations\", and \"media\".", fixed = TRUE ) })