Skip to content

ci: skip live external-reference tests by default in CI - #1002

Draft
mhweber wants to merge 2 commits into
developfrom
ci/skip-live-reference-tests
Draft

ci: skip live external-reference tests by default in CI#1002
mhweber wants to merge 2 commits into
developfrom
ci/skip-live-reference-tests

Conversation

@mhweber

@mhweber mhweber commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

This branch and PR were generated by Copilot based on error in #1001. Add a testthat helper (helper-live.R) that skips tests requiring live downloads unless EPATADA_RUN_LIVE_TESTS=true and internet is available. Call the helper from live-reference tests in test-WQPWQXATTAINSCSTRefs.R. This prevents CI flakes when external downloads fail.

Pull Request Checklist (convert PR to draft if in progress)

Required

  • Update your branch from the latest develop and resolve any merge conflicts

  • Run devtools::test(), devtools::check(), and devtools::document() locally; ensure tests pass and fix any errors, warnings, or notes. Add new dependencies to DESCRIPTION and document appropriately

  • Add/update vignettes for corresponding changes in functionality, list these under articles in _pkgdown.yml, and ensure added/updated vignettes run and build with proper formatting locally

  • Request review from at least one developer team member (convert PR to ready for review if it was designated as in progress)

Best practices

  • Include a summary of the changes made and relevant context/motivation

  • Link issues to auto-close on merge (use Development sidebar or include "Closes #" in the PR)

  • Refresh inline/block comments for clarity

  • Update roxygen docs and include examples; review help pages

  • Add/update tests in tests/testthat; review the bot's coverage report from test-coverage and confirm all changes are covered

Conditional

  • If there is a bot spelling comment, run spelling::spell_check_package() locally and fix any misspellings; add approved project terms to WORDLIST with spelling::update_wordlist()

  • If tests fail suggesting internal reference files need a refresh, run .TADA_UpdateRefFiles() and .TADA_UpdateExampleData() locally via MaintenanceScheduled.R or trigger the Component File Update GitHub Action

  • If new example data files were added, document them in ExampleData.R and include them in MaintenanceScheduled.R for regular refresh

  • If columns were added/updated, update RequiredCols.R

  • If changes affect other package or the shiny app functions, update those impacted functions accordingly

Add a testthat helper (helper-live.R) that skips tests requiring live downloads unless
EPATADA_RUN_LIVE_TESTS=true and internet is available. Call the helper from live-reference
tests in test-WQPWQXATTAINSCSTRefs.R. This prevents CI flakes when external downloads fail.
@github-actions

Copy link
Copy Markdown
Contributor

coverage-report

File Coverage Missing
All files 61%

Minimum allowed coverage is 20%

Generated by 🐒 cobertura-action against a174e1c

@jbousquin

Copy link
Copy Markdown
Collaborator

Quick thoughts:

  • Seems like copilot dropped a lot of unit tests that weren't replaced?
    We should discuss approach/objectives:
  • This seems to skip when internet is down (i.e., user side) whereas what I've observed is server outages. What I've done in other projects is to have a list of base_url for all the systems being pinged and checking for 200. Using that approach to skip tests requires the connection between base_url and function/test, but can be more targeted.
  • Higher level objective is to remove outside factors that are causing tests to fail when our code is doing what it should. Long-term I still think mocking test results is the best way to do that. However, what is lost w/ mocking (real time awareness of changes to services) could be informed by this type of approach (i.e., as a warning instead of a skip), so if it is a short-term fix it's not wasted effort.

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.

3 participants