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

Add Unit Tests #1

Merged
merged 17 commits into from
May 29, 2024
Merged

Add Unit Tests #1

merged 17 commits into from
May 29, 2024

Conversation

njlyon0
Copy link
Member

@njlyon0 njlyon0 commented May 3, 2024

This is just the scaffolding of the unit testing architecture. Opening this PR now just so this task doesn't slip off of either of our radars. See GitHub Enterprise issue #265 for more details

@njlyon0 njlyon0 added the enhancement New feature or request label May 3, 2024
@njlyon0
Copy link
Member Author

njlyon0 commented May 29, 2024

These unit tests look good to me! However, it worries me that some of the GitHub Actions (GHAs) failed on your last push. I did some digging in the GHA details and found this warning/error. Seems like for some reason the lmerTest unit test in stat_extract is having issues.

Doesn't seem like our fault though so my push just now was to check whether commenting out just that bit would cause the GHAs to succeed.

If they do work, I'm happy to green light merging this PR even without that unit test but I'd also support you if you felt like figuring out why it works locally but not on GitHub.

@angelchen7
Copy link
Contributor

Thanks for looking these over, Nick!! The checks were successful locally so I was a bit stumped that the GHAs failed 🤕 getting all these package version to work together in all scenarios can be tricky..

I want to merge the PR but it seems that another GHA is failing due to the example for word_cloud_plot(). The error says " *** caught segfault *** address 0x1, cause 'memory not mapped'" which is very strange and I've never seen that happen before 😖 A quick Google search suggests that it may be due to package versions..

@angelchen7
Copy link
Contributor

In order to pass the GHA checks, I removed word_cloud_plot() example and unit tests. This just didn't seem like something we want to spend a lot of time debugging on right now :/

Then to refine word_cloud_prep() a bit more, I defined more error types for it and wrote some new unit tests to test those errors. By the way, the function already errors out when the text column is provided but it isn't a character type so I did not have to write a new error message for that case. When the text column isn't a character type, tidytext works under-the-hood in the function and spits out an error message that says "Error in check_input(x) : Input must be a character vector of any length or a list of character vectors, each of which has a length of 1."

Everything seem to work locally and the GHA checks all pass so it should be good to go now! I will merge the PR 🎉

@angelchen7 angelchen7 merged commit 2fa10fb into main May 29, 2024
5 checks passed
@angelchen7 angelchen7 deleted the add-tests branch May 29, 2024 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants