-
Notifications
You must be signed in to change notification settings - Fork 26
ci: use with-connect for integration testing against many versions of Connect #739
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
Conversation
|
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
| - "release" # special value that always points to the latest Connect release | ||
| - "2025.09.0" # jammy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing 2025.10.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentionally so. I only have it testing every 6 months, same as on connectapi. I don't think it's worth testing every single release in there.
| - "2022.10.0" # bionic | ||
| name: Integration tests against Connect ${{ matrix.version }} | ||
| env: | ||
| python-version: 3.13 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe worth checking the last 2 or 3 versions of python here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a separate matrix that tests across python versions. I'm not sure we need python versions x Connect versions here, these are just testing that we haven't done anything that breaks the Connect API contract.
| os.environ["CONNECT_SERVER"] = original_server_value | ||
|
|
||
| # noinspection SpellCheckingInspection | ||
| @pytest.mark.skip(reason="Skipping R manifest test (requires R 3.5, docker containers have moved on).") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this test just be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed or deleted, yeah. I made #741 to look into that.
mconflitti-pbc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple comments/suggestions but looks good!
Intent
Relates to #649. Turns out there already were tests that would push to a Connect server, if you had CONNECT_SERVER and CONNECT_API_KEY set in the environment. Perfect for
with-connect.Also bumps some versions and adds python 3.13 to the matrices. And mocks out in the test suite the shinyapps.io thing that pops open a browser after deploying, which is annoying when running the tests.
Approach
Edit some yaml. Also run it locally.
Added a skip for a test for uploading an R app using a manifest because the manifest specifies an old version of R that's not found in our docker images. This should probably get updated at some point, but I don't think that needs to block progress here.
A couple of test fixtures are updated to have a
.python-versionversion range set so they can successfully deploy against a Connect server in docker that is not guaranteed to match what is in the local environment. That feature was only added in Connect 2025.03.0, so those test are also skipped on older Connect.Automated Tests
So much.
Directions for Reviewers
See that the new stuff that runs.
Checklist
rsconnect-python-tests-at-nightworkflow in Connect against this feature branch.