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

Replaced testthat function #3094

Merged

Conversation

meetagrawal09
Copy link
Collaborator

Description

The PR tries to remove use of testthat outside of unit tests and replace it with assertthat
This PR does not replicate efforts made in #3067
PR tries to fix #2987

Motivation and Context

Review Time Estimate

  • Immediately
  • Within one week
  • When possible

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • My name is in the list of CITATION.cff
  • I have updated the CHANGELOG.md.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@meetagrawal09
Copy link
Collaborator Author

@Aariq I would like to know if there are any other potential places where the change is needed

@Aariq
Copy link
Collaborator

Aariq commented Jan 24, 2023

@Aariq I would like to know if there are any other potential places where the change is needed

I don't know. In RStudio you can use "Find in files ..." in the Edit menu to search for testthat or maybe expect_

@meetagrawal09 meetagrawal09 changed the title Replaced testhatt function Replaced testthat function Jan 24, 2023
@meetagrawal09
Copy link
Collaborator Author

I don't know. In RStudio you can use "Find in files ..." in the Edit menu to search for testthat or maybe expect_

Followed this. There are no other places which need to be updated.

@meetagrawal09
Copy link
Collaborator Author

Can you guide me what further changes need to be made ( or commands need to be run ) in-order to make the checks pass and if the change made to this function seems correct ?

@Aariq
Copy link
Collaborator

Aariq commented Jan 24, 2023

I think maybe you need to run this script: https://github.com/PecanProject/pecan/blob/develop/scripts/generate_dependencies.R

@meetagrawal09 meetagrawal09 force-pushed the replace-testhatt-functions branch from f3ef964 to f5e7056 Compare January 25, 2023 03:03
@meetagrawal09
Copy link
Collaborator Author

meetagrawal09 commented Jan 30, 2023

This PR covers all the instances of testthat being used outside of unit testing which have not already been a part of #3067
The work on this PR is complete and it is ready for a review

@mdietze mdietze merged commit 353b4dd into PecanProject:develop Jan 30, 2023
@meetagrawal09 meetagrawal09 deleted the replace-testhatt-functions branch January 31, 2023 01:55
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.

Don't use testthat inside of functions
3 participants