Skip to content

Update to testthat 3#201

Open
olivroy wants to merge 2 commits into
eliocamp:mainfrom
olivroy:dep
Open

Update to testthat 3#201
olivroy wants to merge 2 commits into
eliocamp:mainfrom
olivroy:dep

Conversation

@olivroy

@olivroy olivroy commented Jan 29, 2025

Copy link
Copy Markdown
Contributor

Also removes unused suggests

add a search bar to website also

The added imports don't change anything to the package, since they are already imported by ggplot2.

I wanted to see if I could remove the {plyr} dependency, but I couldn't. The PR is already big as it is.

@olivroy

olivroy commented Jun 3, 2025

Copy link
Copy Markdown
Contributor Author

Hi @eliocamp ! Let me know what you think or if you need more explanation / PR split

@eliocamp

Copy link
Copy Markdown
Owner

Hi! Yes, it is a pretty large PR that does a lot of different stuff. It would be great if you could split it into more atomic PRs.

olivroy added 2 commits June 25, 2025 11:56
(I know the diff is bad from GitHub, but I verified that they matched when I first opened the PR.
@olivroy olivroy changed the title Update to testthat 3, avoid partial match, update ggplot2 code Update to testthat 3 Jun 25, 2025
@olivroy

olivroy commented Jun 25, 2025

Copy link
Copy Markdown
Contributor Author

I modified this PR to be only testthat 3. Opened another one for partial matching. I will see if I re-implement the other changes. The move to testthat 3 may help with future proofing the package

@eliocamp

eliocamp commented Jul 1, 2025

Copy link
Copy Markdown
Owner

I'm not versed in snapshot testing. The expect_snapshot() expectation test for the printout. But in many cases what the test was testing was the object. Shouldn't it be expect_snapshot_value() instead?

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