Skip to content

Testing with the openeo-test-api#166

Merged
m-mohr merged 21 commits intoOpen-EO:masterfrom
niebl:ci_tests
Feb 12, 2026
Merged

Testing with the openeo-test-api#166
m-mohr merged 21 commits intoOpen-EO:masterfrom
niebl:ci_tests

Conversation

@niebl
Copy link
Copy Markdown
Contributor

@niebl niebl commented Feb 4, 2026

Changes:

  • Change the backend connection tests to use the openeo-test-api
  • CI tests for the backend connection tests
  • Instructions for local testing with the openeo-test-api

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds continuous integration testing infrastructure using the openeo-test-api, enabling automated testing of the R client against a local test backend. The changes modify test configurations to run against localhost instead of production endpoints and introduce a new GitHub Actions workflow.

Changes:

  • New GitHub Actions workflow that sets up and runs tests against openeo-test-api on localhost
  • Modified test files to use localhost endpoint (http://127.0.0.1:8080) instead of production URLs
  • Updated RoxygenNote version from 7.3.2 to 7.3.3

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 9 comments.

File Description
.github/workflows/tests.yml New CI workflow that sets up R, Node.js, installs openeo-test-api, creates test user, and runs tests
tests/testthat/test-gee-test.R Commented out skip statement and changed endpoint from production to localhost with hardcoded test credentials
tests/testthat/test-client.R Changed connection test endpoint from openeo.cloud to localhost
DESCRIPTION Updated RoxygenNote version from 7.3.2 to 7.3.3

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@m-mohr
Copy link
Copy Markdown
Member

m-mohr commented Feb 5, 2026

Additional task: #167

@niebl niebl force-pushed the ci_tests branch 2 times, most recently from 505c5c8 to 762278b Compare February 5, 2026 13:12
@m-mohr m-mohr linked an issue Feb 5, 2026 that may be closed by this pull request
@flahn
Copy link
Copy Markdown
Member

flahn commented Feb 5, 2026

Hi, thanks for the pull request, but regarding the skipped tests, those are required to be skipped. When you send in a package at CRAN the package is build, the tests are run and some other documentation tests are done. In the package build and test stage at CRAN we have neither access to the host nor to internet resources. The tests are just reproducible and somewhat simple unit tests. In the past I have run those more complex tests on my local machine, before releasing the package to CRAN.

@m-mohr
Copy link
Copy Markdown
Member

m-mohr commented Feb 9, 2026

@niebl Have you updated the PR according to Florians comments?

@m-mohr
Copy link
Copy Markdown
Member

m-mohr commented Feb 9, 2026

To be more concrete here: Ideally we run the skipped tests in CI, if possible, but the tests would be skipped in the CRAN checks.

@niebl
Copy link
Copy Markdown
Contributor Author

niebl commented Feb 9, 2026

Oh I understand now. in that case we should go back to the test.yml.

These CI tests can however take up to half an hour to run.

@m-mohr
Copy link
Copy Markdown
Member

m-mohr commented Feb 9, 2026

Why are the tests so slow? Are these some specific tests that do processing? Most tasks should be rather fast.

@niebl
Copy link
Copy Markdown
Contributor Author

niebl commented Feb 9, 2026

the most overhead comes from the CI environment setup: Installing R and installing R-dependencies

Locally running the tests leads to a lot more immediate results

@niebl
Copy link
Copy Markdown
Contributor Author

niebl commented Feb 10, 2026

@flahn would the use of skip_on_cran() work in this case too? that could allow us to create a CI action for testing backend connections without interfering with CRAN

@niebl niebl changed the title Ci tests using the openeo-test-api Testing with the openeo-test-api Feb 10, 2026
@m-mohr m-mohr requested a review from flahn February 10, 2026 16:58
@flahn
Copy link
Copy Markdown
Member

flahn commented Feb 10, 2026

I think I had been at this point in the past. I skipped the test completely instead only on CRAN, because before sending the package to CRAN I usually also run devtools::check_win_devel() which also runs the build, tests and checks on an isolated machine, which is not the CRAN environment and the test will fail.

However I really like the idea of the CI pipeline, but maybe we should think the other way around. In which circumstances do we want to run the test? Looking at the testthat page the function skip_if_not() comes to my mind in combination with Sys.getenv(). This allows us to fetch a specific OS environment variable which we can setup in a stage of the the CI pipeline. That way it is neither run on CRAN nor on the win_devel environment. Might solve it, what do you think?

@niebl
Copy link
Copy Markdown
Contributor Author

niebl commented Feb 11, 2026

I think I had been at this point in the past. I skipped the test completely instead only on CRAN, because before sending the package to CRAN I usually also run devtools::check_win_devel() which also runs the build, tests and checks on an isolated machine, which is not the CRAN environment and the test will fail.

However I really like the idea of the CI pipeline, but maybe we should think the other way around. In which circumstances do we want to run the test? Looking at the testthat page the function skip_if_not() comes to my mind in combination with Sys.getenv(). This allows us to fetch a specific OS environment variable which we can setup in a stage of the the CI pipeline. That way it is neither run on CRAN nor on the win_devel environment. Might solve it, what do you think?

That should definitely work better and has less risk of breaking workflows. I have pushed a commit that includes the according changes.

The only issue I see is that testing within RStudio will skip the tests in question, but I don't believe that to be a significant problem as running those tests manually with the environment variable via CONNECTION_TESTS=true R -f testthat.R works just as well

Copy link
Copy Markdown
Member

@flahn flahn left a comment

Choose a reason for hiding this comment

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

lgtm

@niebl niebl force-pushed the ci_tests branch 2 times, most recently from 1d26a9c to eb070ce Compare February 12, 2026 13:59
@m-mohr m-mohr merged commit 8ab9e0a into Open-EO:master Feb 12, 2026
7 checks passed
@m-mohr m-mohr added this to the 1.5.0 milestone Mar 1, 2026
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.

Add documentation how to run tests

4 participants