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

Hrm test #537

Merged
merged 33 commits into from
Oct 31, 2024
Merged

Hrm test #537

merged 33 commits into from
Oct 31, 2024

Conversation

hillarymarler
Copy link
Collaborator

@hillarymarler hillarymarler commented Oct 29, 2024

This is an option to automate checking URLs in the package as a testthat test. I corrected any URLs that had been moved/redirected. There were a couple of URLs I excluded from the test:

"https://www.itecmembers.org/attains/" - something about its server configuration is not allowing it to send the correct HTTP status code. I tried a couple of different methods and could not get any response, though the site loads fine if you paste the link in a browser.

And the http and https links to cran as they always come back with a redirect message (I think due to automatic redirect to a mirror). I can modify the test to accept/not flag redirects (300s) if that is preferable for our use case. Currently, the test looks for 200 response and anything other than that will fail.

If the test were to fail, the easiest way to find the failing URL would be to open the package and manually run lines 3-53 in the test-URLChecker.R file. The "df_false" data frame will contain any failing urls and their respective http response codes.

@hillarymarler hillarymarler linked an issue Oct 29, 2024 that may be closed by this pull request
Removed unnecessary package (urlchecker)
@hillarymarler
Copy link
Collaborator Author

Unfortunately, the new test failed here (though not while I was testing it). I will work on getting it fixed so it can be reviewed.

Potential bug fix, switched to filter on partial string "200"
@cristinamullin
Copy link
Collaborator

Great work Hillary! Redirects (300s) should be fine for our our use case - do you want to modify the code to accept those now or wait to see if it becomes burdensome to edit the redirects in the future?

I think it'd be helpful to add directions (below) on how to fix the broken urls to Maintenance.R. The code / any notes in Maintenance.R will need to be commented out (like the styler code in there now), but I think it's helpful to have all this in one spot.

@hillarymarler
Copy link
Collaborator Author

Sure - I can modify so that redirects are accepted too. This would mean I can remove CRAN from the exclusion list and exclude only the ITEC site from the test. I can also add instructions on how to identify and fix broken links if this test fails in Maintenance.R.

Added instructions to Maintenance.R, updated URLChecker.R to allow 301 and 302 response codes, updated TADA Shiny link
@cristinamullin cristinamullin merged commit 265ff53 into develop Oct 31, 2024
7 checks passed
@cristinamullin cristinamullin deleted the hrm_test branch October 31, 2024 14:24
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.

Automate broken link check
2 participants