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

Check for other package messaging found in our snapshots #116

Closed
ddsjoberg opened this issue Apr 15, 2024 · 2 comments · Fixed by #125
Closed

Check for other package messaging found in our snapshots #116

ddsjoberg opened this issue Apr 15, 2024 · 2 comments · Fixed by #125
Assignees

Comments

@ddsjoberg
Copy link
Collaborator

Our snapshot tests shouldn't include messages from other packages. We don't want a small change in wording from a 3rd party package lead to a breakage in cardx. Note that this would NOT cause a breaking change from CRAN's perspective, because snapshot tests are not run on CRAN

@zdz2101 zdz2101 self-assigned this Apr 16, 2024
@zdz2101
Copy link
Contributor

zdz2101 commented Apr 16, 2024

  • ard_aod_wald_test.md : nothing found
  • ard_car_anova.md : potentially something found
  • ard_car_vif.md : error column has other package messaging
  • ard_effectsize_cohens_d.md : error column has other package messaging
  • ard_effectsize_hedges_g.md : error column has other package messaging
  • ard_proportion_ci.md : nothing found
  • ard_regression_basic.md : nothing found
  • ard_regression.md : nothing found
  • ard_stats_aov.md : nothing found
  • ard_stats_chisq_test.md : nothing found
  • ard_stats_kruskal_test.md : nothing found
  • ard_stats_mood_test.md : nothing found
  • ard_stats_oneway_test.md : nothing found
  • ard_stats_t_test.md : error column has other package messaging
  • ard_survey_svychisq.md : nothing found
  • ard_survey_svycontinuous.md : nothing found
  • ard_survey_svyranktest.md : nothing found
  • ard_survival_survfit.md : nothing found
  • proportion_ci.md: nothing found

@zdz2101 zdz2101 linked a pull request Apr 18, 2024 that will close this issue
13 tasks
@zdz2101
Copy link
Contributor

zdz2101 commented Apr 23, 2024

@ddsjoberg found the tests/snapshots that contain error messaging from other packages, is solution to simply "unselect" the column in the snapshot?

ddsjoberg pushed a commit that referenced this issue Apr 25, 2024
**What changes are proposed in this pull request?**
Check and remove snapshots that contain other-package messaging

Provide more detail here as needed.

Closes #116



--------------------------------------------------------------------------------

Pre-review Checklist (if item does not apply, mark is as complete)
- [x] **All** GitHub Action workflows pass with a ✅
- [x] PR branch has pulled the most recent updates from master branch:
`usethis::pr_merge_main()`
- [x] If a bug was fixed, a unit test was added.
- [x] If a new `ard_*()` function was added, it passes the ARD
structural checks from `cards::check_ard_structure()`.
- [x] If a new `ard_*()` function was added, `set_cli_abort_call()` has
been set.
- [x] If a new `ard_*()` function was added and it depends on another
package (such as, `broom`), `is_pkg_installed("broom", reference_pkg =
"cardx")` has been set in the function call and the following added to
the roxygen comments: `@examplesIf
do.call(asNamespace("cardx")$is_pkg_installed, list(pkg = "broom"",
reference_pkg = "cardx"))`
- [x] Code coverage is suitable for any new functions/features
(generally, 100% coverage for new code): `devtools::test_coverage()`

Reviewer Checklist (if item does not apply, mark is as complete)

- [x] If a bug was fixed, a unit test was added.
- [x] Code coverage is suitable for any new functions/features:
`devtools::test_coverage()`

When the branch is ready to be merged:
- [x] Update `NEWS.md` with the changes from this pull request under the
heading "`# cardx (development version)`". If there is an issue
associated with the pull request, reference it in parentheses at the end
update (see `NEWS.md` for examples).
- [x] **All** GitHub Action workflows pass with a ✅
- [x] Approve Pull Request
- [x] Merge the PR. Please use "Squash and merge" or "Rebase and merge".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants