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

tests #44

Merged
merged 23 commits into from
Nov 30, 2021
Merged

tests #44

merged 23 commits into from
Nov 30, 2021

Conversation

TSchiefer
Copy link
Member

closes #31

Copy link
Collaborator

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

Thanks, looks good!

DESCRIPTION Outdated
@@ -47,6 +47,7 @@ Imports:
Suggests:
daff,
knitr,
pkgload,
Copy link
Collaborator

Choose a reason for hiding this comment

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

What do we need this for?

Copy link
Member Author

Choose a reason for hiding this comment

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

R CMD check produces a warning otherwise, cause pkgload::load_all() is used in example/swc_get_mapping.R

Copy link
Collaborator

Choose a reason for hiding this comment

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

The example file shouldn't use it either.

tests/testthat/test-internal-consistency.R Outdated Show resolved Hide resolved
tests/testthat/test-up-to-date.R Outdated Show resolved Hide resolved
@TSchiefer
Copy link
Member Author

think the Windows tests should now work with 4edd1bb -- currently failing due to https://chocolatey.org/api/v2/ being down for maintenance, checking later again.

@TSchiefer
Copy link
Member Author

TSchiefer commented Mar 31, 2021

this seems ok now to me.
I disabled tests/testthat/test-up-to-date.R on Windows, since it seems to have been that way also before it got broken completely.
Now it again failed on Windows (e.g. https://github.com/cynkra/munch/pull/44/checks?check_run_id=2236574313), which I don't understand (or does it maybe have something to do with weird Windows-encoding?). Maybe we can fix it somehow or leave it disabled on Windows.

@krlmlr krlmlr mentioned this pull request Mar 31, 2021
@TSchiefer
Copy link
Member Author

I removed the obvious dependencies on {pkgload}; now I can't find any mention of {pkgload} anywhere, but the tests are still complaining that an apparent dependency isn't acknowledged.

@krlmlr
Copy link
Collaborator

krlmlr commented Mar 31, 2021

It's tricky.

PRs are checked using a synthetic commit where the source branch is merged into the target branch: https://github.com/cynkra/munch/pull/44/checks?check_run_id=2237415606#step:3:394. The main branch was ahead and contained the evil pkgload line.

I think it's fine to use pkgload in the data-raw/ directory.

@krlmlr
Copy link
Collaborator

krlmlr commented Mar 31, 2021

To recover, git reset --hard f809e25, force-push, and redo.

@krlmlr
Copy link
Collaborator

krlmlr commented Mar 31, 2021

We really need to merge main into this branch to avoid losing our minds.

@TSchiefer
Copy link
Member Author

TSchiefer commented Mar 31, 2021

Sorry for the mess... when I locally merge master main into this branch, no changes are the result. So I will let these checks finish, cause I haven't lost all hope that this will work now...

@krlmlr
Copy link
Collaborator

krlmlr commented Mar 31, 2021

The main branch is called main.

@krlmlr
Copy link
Collaborator

krlmlr commented Mar 31, 2021

...in this repo.

@TSchiefer
Copy link
Member Author

you're right, I meant main; seems to be good now.

@krlmlr krlmlr merged commit 5973a21 into main Nov 30, 2021
@krlmlr krlmlr deleted the f-31-tests branch November 30, 2021 08:41
@krlmlr
Copy link
Collaborator

krlmlr commented Nov 30, 2021

Thanks!

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 1, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Tests
2 participants