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

Use testthat 3 #1180

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Use testthat 3 #1180

wants to merge 8 commits into from

Conversation

olivroy
Copy link
Contributor

@olivroy olivroy commented Dec 4, 2023

I also silenced a couple of tests / output along the way.

Copy link
Collaborator

@simonpcouch simonpcouch left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! This is a lot of work; I appreciate you putting the time in here.

Could you separate the test changes, vectorized rlang::is_installed(), name_repair silencing, and transitions from & to && in internals into separate PRs? That will help me review more informatively.

R/stats-kmeans-tidiers.R Outdated Show resolved Hide resolved
Merge commit 'ac745681daebddaaf3c5dda45e535a7fc98314e3'

#Conflicts:
#	R/btergm-tidiers.R
#	R/muhaz-tidiers.R
#	man/glance.muhaz.Rd
#	man/tidy.btergm.Rd
#	man/tidy.muhaz.Rd
@olivroy
Copy link
Contributor Author

olivroy commented Dec 5, 2023

Thanks for triggering CI. As you see, there is still a lot of noise in the logs. I will try silencing most of it. It is mostly due to the fact that packages loaded in broom's tests are quite noisy on load. (when clicking on show testthat output)

@simonpcouch
Copy link
Collaborator

Noise in the logs is okay! What's more important is my ability to understand the change set comprehensively. As-is, there are still many changes that seem unrelated to the switch to testthat 3e—I'd welcome those other changes in separate PRs where their scope is more clearly defined, but will ask you to further pare back the changes here to only those needed to related to switching to 3e.

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.

None yet

2 participants