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

Switch test expect_snapshot to explicit expect_warning #48

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vejnar
Copy link

@vejnar vejnar commented Nov 26, 2023

Problem: In recent R versions (not sure when it changed), the format of warnings has changed making the snapshot _snaps/utils.md incorrect and the test to fail.

This PR switches the test to explicit expect_warning to catch the warning and then expect_equal. This way it should pass with any R version.

Please comment or merge.

@gaborcsardi
Copy link
Member

making the snapshot _snaps/utils.md incorrect and the test to fail.

It does not fail for me, on R-4.3.2 or R-devel. OTOH testthat has changed the output slightly, try updating testthat.

@vejnar
Copy link
Author

vejnar commented Nov 27, 2023

I am using testthat 3.2.0 and R 4.3.2 and it fails.

I am thinking it would be better to have more explicit tests, ie not relying on specific output formatting, as proposed in this PR so tests are passing with any versions, past or present.

@gaborcsardi
Copy link
Member

Since we rely on snapshot tests a lot, it is important to understand why it fails for you if it does not on any of our CI platforms. What is your platform?

@vejnar
Copy link
Author

vejnar commented Nov 27, 2023

I am on Archlinux. I am building packages using the AUR for all R packages.

@gaborcsardi
Copy link
Member

If you are running R CMD check, then the snapshot test failures are ignored by default, so it should not be a problem.

But I actually reproduce the failure on Archlinux:

FROM archlinux:latest
WORKDIR /root
RUN pacman -Sy --noconfirm r gcc which git make
RUN git clone https://github.com/r-lib/rappdirs.git
RUN R -q -e 'source("https://pak.r-lib.org/install.R")'
WORKDIR /root/rappdirs
RUN R -q -e 'pak::pkg_install("testthat")'
RUN R -q -e 'testthat::test_local()'

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.

2 participants