ci: Use `with-connect` action for integration tests #475

nealrichardson · 2025-10-25T18:06:50Z

Intent

Remove HackyConnect and the related docker compose tooling for running integration tests, in favor of a new with-connect utility that @tdstein started work on.

The main purpose for doing this here is to prove out how with-connect and its accompanying GitHub Action can be used in CI, so that we can roll it out to the range of other open-source packages that need it. The code deletion here is a nice benefit.

In the process, I discovered that the v1 git endpoints weren't operational in 2022.10.0, as we thought in #459, but rather in 2022.12.0, so I updated those version checks.

Fixes #446
Fixes #240 (as best we can)
Fixes #231 (in a way)

Approach

Iteration back and forth here and on https://github.com/nealrichardson/with-connect/tree/dev, my fork of the action. Once everything was switched over and confirmed working, delete as much as I could, including some stale local dev setup and a test file that was all skips.

There is some further simplification of the integration test suite to do, but I've deferred that here.

Checklist

Does this change update NEWS.md (referencing the connected issue if necessary)?
Does this change need documentation? Have you run devtools::document()?

nealrichardson · 2025-10-26T18:45:03Z

.github/workflows/integration-tests.yaml

      fail-fast: false
      matrix:
        version:
+          - "2025.09.0" # jammy


I think we can also add "latest" here and that will solve #240, if we want.

Actually, latest doesn't get updated! It points at 2022.08.0. But jammy tracks the most recent release. I thought we had a nightly container build but I guess not. Until then, at least jammy will make sure we have the latest release in the matrix.

nealrichardson · 2025-10-26T18:45:43Z

.github/workflows/integration-tests.yaml

-          - "2022.03.2"
-          - "2021.09.0"
-          - "1.8.8.2"
+          - "2022.10.0" # bionic


Because with-connect relies on the bootstrap endpoint to set up the first user, this is now the minimum version we can test. It's 3 years ago though, so I think that's fine.

nealrichardson · 2025-10-26T18:46:18Z

.github/workflows/integration-tests.yaml

        with:
-          extra-packages: any::rcmdcheck, local::.
-          needs: check
+          extra-packages: local::.


Don't need rcmdcheck because we're just invoking the test file. We don't need to do R CMD check here, we do that in the other jobs.

Thanks for cleaning this up

.github/workflows/integration-tests.yaml

nealrichardson · 2025-10-26T18:48:58Z

.github/workflows/pkgdown.yaml

+                TEST_1_SERVER=Sys.getenv("CONNECT_SERVER"),
+                TEST_1_API_KEY=Sys.getenv("CONNECT_API_KEY")
+              )
+              pkgdown::build_site_github_pages(new_process = FALSE, install = FALSE)'


In a followup that will remediate the TEST_1_SERVER usage, I'll check for how exactly pkgdown needs a live Connect server, not sure it does or should. (But maybe it's fine?)

nealrichardson · 2025-10-26T18:49:21Z

.github/workflows/pr-commands.yaml

 name: Commands

 jobs:
-  update_snapshot:


I don't think this is ever used.

…passing

tdstein · 2025-10-27T12:40:49Z

.github/CONTRIBUTING.md

- By default, this will run against a contemporary version of Connect. To test against an older version, set the environment variable `CONNECT_VERSION` to something else and then run `build_test_env()`.
- Set `CONNECTAPI_INTEGRATED=true` in the environment to enable running the integration tests (they're skipped by default).
- Run them with `source("tests/test-integrated.R")`
+- Get the [with-connect](https://github.com/nealrichardson/with-connect/blob/dev/README.md) tool


tdstein

Hooray! This is awesome!

.github/workflows/integration-tests.yaml

zackverham · 2025-10-27T13:24:05Z

(lurking observation: this is excellent)

tdstein

In the integration test runs, I see the following:


Problem talking to Posit Connect at http://localhost:3939/__api__/server_settings
Invalid (empty) API key. Please provide a valid API key
Problem talking to Posit Connect at http://localhost:3939/__api__/server_settings
http://localhost:3939/__api__/server_settings request failed with Client error: (401) Unauthorized (code: 30, error: We couldn't log you in with the provided credentials. Please ask your administrator for assistance.)

Is this expected?

nealrichardson · 2025-10-27T16:22:01Z

In the integration test runs, I see the following:


Problem talking to Posit Connect at http://localhost:3939/__api__/server_settings
Invalid (empty) API key. Please provide a valid API key
Problem talking to Posit Connect at http://localhost:3939/__api__/server_settings
http://localhost:3939/__api__/server_settings request failed with Client error: (401) Unauthorized (code: 30, error: We couldn't log you in with the provided credentials. Please ask your administrator for assistance.)

Is this expected?

I suspected so since the rest of the tests succeed. It looks like a sloppy test: when .check_is_fatal = FALSE, it prints the error instead of raising it, and that output isn't being caught on https://github.com/posit-dev/connectapi/blob/main/tests/integrated/test-connect.R#L52-L56

tdstein

Looks good to me!

toph-allen

This looks awesome @nealrichardson, thank you for integrating this! Also @tdstein! 👏🏻

I had thought some other tools used HackyConnect but a cursory GitHub search doesn't actually find anything using this, as far as I can tell.

toph-allen · 2025-10-27T18:55:24Z

.github/workflows/pkgdown.yaml

+        with:
+          # Runs on the default version in the with-connect action
+          license: ${{ secrets.CONNECT_LICENSE_FILE }}
+          # TODO: rewrite tests to use CONNECT_* env vars directly


Maybe reference the issue in this line, or just remove it since we have the issue?

Suggested change

# TODO: rewrite tests to use CONNECT_* env vars directly

[ # TODO: rewrite tests to use CONNECT_* env vars directly (issue #476)

Morally I agree, but I'm just going to do this once this PR lands, so I won't forget

toph-allen · 2025-10-27T19:00:34Z

.github/workflows/test-coverage.yaml

-            token = if (token != "") token
-          )
-        shell: Rscript {0}
+          CONNECTAPI_INTEGRATED: "true"


My assumption is that our coverage tests need to use Connect because the tests need to actually run to determine what parts of the code are covered?

Correct, the coverage stats are computed based on running both the unit+mock tests and the integration tests.

nealrichardson · 2025-10-27T20:36:51Z

This looks awesome @nealrichardson, thank you for integrating this! Also @tdstein! 👏🏻

I had thought some other tools used HackyConnect but a cursory GitHub search doesn't actually find anything using this, as far as I can tell.

I've seen copies/translations of it in at least pins-python, and I thought other places--but I'm coming for them all now.

nealrichardson · 2025-10-27T21:14:07Z

pins-r integration tests use HackyConnect indirectly in order to create users and get their API keys: https://github.com/rstudio/pins-r/blob/main/.github/workflows/posit-connect.yaml#L41-L44

This can be rewritten without HackyConnect, but will take some refactoring.

In the short run, that CI job could be touched up to install an old version of connectapi that has HackyConnect, if that's needed. But we should just fix it.

nealrichardson added 14 commits October 25, 2025 14:06

Try with-connect

117846d

Delete stale local stuff

3648913

Map env vars to what the tests currently expect

68e6c4e

use rcmdcheck to handle build + check

dc6dd77

Set check output dir

d118dde

Just run the integration tests, no check

e53a42f

Set the env vars within R

6137c4c

Sigh

f6eb1c9

Update the other workflows, add line breaks

1712079

Remove HackyConnect et al.

e98bb82

composite actions don't support yaml mappings

eb99b3b

Fix collate

a64161c

oops, restore codecov token

35c9a2b

more cleanup

e32ac72

nealrichardson marked this pull request as ready for review October 26, 2025 18:42

nealrichardson commented Oct 26, 2025

View reviewed changes

.github/workflows/integration-tests.yaml Outdated Show resolved Hide resolved

nealrichardson commented Oct 26, 2025

View reviewed changes

.github/workflows/pr-commands.yaml

name: Commands

jobs:

update_snapshot:

Copy link

Collaborator Author

nealrichardson Oct 26, 2025

Choose a reason for hiding this comment

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

I don't think this is ever used.

nealrichardson changed the title ~~ci: Try with-connect action~~ ci: Use with-connect action for integration tests Oct 26, 2025

nealrichardson added 5 commits October 26, 2025 17:01

Add 'latest', try env

0aef98d

Poke around git v1 API availability

3e16050

Try 2022.12

e11e33e

Fix latest tag

066f11f

Update v1 git functions based on the actual version where they start …

b845160

…passing

tdstein reviewed Oct 27, 2025

View reviewed changes

.github/workflows/integration-tests.yaml Outdated Show resolved Hide resolved

Remove connect prefix from args

ca8634f

nealrichardson requested review from tdstein and toph-allen October 27, 2025 13:15

tdstein reviewed Oct 27, 2025

View reviewed changes

tdstein approved these changes Oct 27, 2025

View reviewed changes

nealrichardson mentioned this pull request Oct 27, 2025

refactor: Update tests and docs to use standard env vars #476

Closed

toph-allen approved these changes Oct 27, 2025

View reviewed changes

nealrichardson merged commit 690bb8c into main Oct 27, 2025
21 checks passed

nealrichardson deleted the with-connect branch October 27, 2025 20:38

This was referenced Oct 28, 2025

ci: rewrite Connect integration tests using with-connect Action rstudio/pins-r#883

Closed

chore: Reformat with air rstudio/pins-r#885

Merged

nealrichardson mentioned this pull request Nov 7, 2025

Use with-connect in all client libraries we maintain posit-dev/with-connect#27

Open

8 tasks

	# TODO: rewrite tests to use CONNECT_* env vars directly
	[ # TODO: rewrite tests to use CONNECT_* env vars directly (issue #476)

ci: Use with-connect action for integration tests #475

ci: Use with-connect action for integration tests #475

Uh oh!

Conversation

nealrichardson commented Oct 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Intent

Approach

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tdstein left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zackverham commented Oct 27, 2025

Uh oh!

tdstein left a comment

Choose a reason for hiding this comment

Uh oh!

nealrichardson commented Oct 27, 2025

Uh oh!

tdstein left a comment

Choose a reason for hiding this comment

Uh oh!

toph-allen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nealrichardson commented Oct 27, 2025

Uh oh!

Uh oh!

nealrichardson commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ci: Use `with-connect` action for integration tests #475

ci: Use `with-connect` action for integration tests #475

nealrichardson commented Oct 25, 2025 •

edited

Loading

nealrichardson commented Oct 27, 2025 •

edited

Loading