Skip to content

Remove redundant tests - #1289

Open
jmgirard wants to merge 1 commit into
mainfrom
trim-redundant-tests
Open

jmgirard wants to merge 1 commit into
mainfrom
trim-redundant-tests

Conversation

@jmgirard

Copy link
Copy Markdown
Contributor

Several of my recent PRs added more tests than the changes needed: checking for rare edge cases, the same check repeated across model classes or inputs, extra model fits, and long comments. Sorry for the extra review load and CI time this caused.

This PR removes the redundant tests. Each fixed behavior keeps one regression test, and no R code changes.

Comment on lines 61 to 68
expected <- .refit_cluster_draws(
model,
data,
group,
20,
iterations,
42,
sim = sim,
relabel = relabel
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[air] reported by reviewdog 🐶

Suggested change
expected <- .refit_cluster_draws(
model,
data,
group,
20,
iterations,
42,
sim = sim,
relabel = relabel
)
expected <- .refit_cluster_draws(model, data, group, iterations, 42, relabel = relabel)

verbose = FALSE
))
}
expect_no_condition(model_parameters(m_single, standardize = "std.nox", verbose = FALSE))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[air] reported by reviewdog 🐶

Suggested change
expect_no_condition(model_parameters(m_single, standardize = "std.nox", verbose = FALSE))
expect_no_condition(model_parameters(
m_single,
standardize = "std.nox",
verbose = FALSE
))

out <- model_parameters(m_single, standardize = "latent")
expect_true(.has_footer(out, " (type: std.lv)."))
expect_false(.has_footer(model_parameters(m_single)))
expect_false(.has_footer(model_parameters(m_single, standardize = TRUE, verbose = FALSE)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[air] reported by reviewdog 🐶

Suggested change
expect_false(.has_footer(model_parameters(m_single, standardize = TRUE, verbose = FALSE)))
expect_false(.has_footer(model_parameters(
m_single,
standardize = TRUE,
verbose = FALSE
)))

cells <- lapply(strsplit(rows, "|", fixed = TRUE), trimws)
header <- cells[[1]]
body <- cells[-1]
expect_identical(vapply(body, function(x) x[header == "p.1"], character(1)), c("a", "b", "c"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[air] reported by reviewdog 🐶

Suggested change
expect_identical(vapply(body, function(x) x[header == "p.1"], character(1)), c("a", "b", "c"))
expect_identical(
vapply(body, function(x) x[header == "p.1"], character(1)),
c("a", "b", "c")
)

mp <- model_parameters(pw, p_adjust = method)
expect_equal(mp$p, stats::p.adjust(p_raw, methods[[method]]), tolerance = 1e-10)
}
expect_false(any(grepl("p-value adjustment method", capture.output(print(mp)), fixed = TRUE)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[air] reported by reviewdog 🐶

Suggested change
expect_false(any(grepl("p-value adjustment method", capture.output(print(mp)), fixed = TRUE)))
expect_false(any(grepl(
"p-value adjustment method",
capture.output(print(mp)),
fixed = TRUE
)))

out <- capture.output(print(mp))
expect_true(any(startsWith(out, "Parameter")))
expect_false(any(grepl("p-value adjustment method", out, fixed = TRUE)))
expect_false(any(grepl("p-value adjustment method", capture.output(print(mp)), fixed = TRUE)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[air] reported by reviewdog 🐶

Suggested change
expect_false(any(grepl("p-value adjustment method", capture.output(print(mp)), fixed = TRUE)))
expect_false(any(grepl(
"p-value adjustment method",
capture.output(print(mp)),
fixed = TRUE
)))

out <- capture.output(print(mp))
expect_true(any(startsWith(out, "contrast")))
expect_false(any(grepl("p-value adjustment method", out, fixed = TRUE)))
expect_false(any(grepl("p-value adjustment method", capture.output(print(mp)), fixed = TRUE)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[air] reported by reviewdog 🐶

Suggested change
expect_false(any(grepl("p-value adjustment method", capture.output(print(mp)), fixed = TRUE)))
expect_false(any(grepl(
"p-value adjustment method",
capture.output(print(mp)),
fixed = TRUE
)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[air] reported by reviewdog 🐶

.refit_cluster_draws <- function(
model,
data,
group,
iterations,
seed,
relabel = group
) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[air] reported by reviewdog 🐶

expect_identical(out$Parameter, c("(Intercept)", "grp(15,20]", "grp(20,50]"))
})
test_that("format_parameters, colon inside function arguments", {
# e.g. plm::pgmm() terms like lag(log(emp), 1:2)
f <- function(x, k) outer(x, k, "^")
model <- lm(mpg ~ f(log(hp), 1:2) * vs + log(wt) * am, data = mtcars)
types <- parameters_type(model)
lag_terms <- c("f(log(hp), 1:2)1", "f(log(hp), 1:2)2")
expect_identical(types$Type[types$Parameter %in% lag_terms], c("unknown", "unknown"))
expect_identical(types$Type[types$Parameter == "f(log(hp), 1:2)1:vs"], "interaction")
fp <- format_parameters(model)
expect_identical(fp[["f(log(hp), 1:2)1"]], "f(log(hp), 1:2)1")
expect_identical(fp[["f(log(hp), 1:2)1:vs"]], "f(log(hp), 1:2)1 * vs")
})
}
)

This branch has not been deployed

No deployments
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.

1 participant