-
Notifications
You must be signed in to change notification settings - Fork 26
ci: add to the integration testing matrix #396
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
base: main
Are you sure you want to change the base?
Conversation
@toph-allen I see #308, is that why these are failing? It's hard to tell--I made #397 to make the tests quieter so it's easier to tell what is failing and what is just making noise. |
@nealrichardson I'm pretty sure that's the reason, yeah — I'm looking at the logs now. The error we get is Basically (I think), the manifests for the test content specify R 3.6.3, which is not present on the Docker images, so Connect fails to build those content items. To get things working, you'll need to spin up Those new R versions would also need to be present on the older images, too, I think (because we only have one manifest per content item for the entire matrix), so you may wind up needing to remove some old Connect versions from the matrix. Let me know if I didn't explain any of that clearly! |
Clearly explained! But 🤮 ! Let's see if we can find a better way. The apps/reports are simple themselves, maybe we can generate manifests on the fly somehow. Alternatively, we could refactor the tests so that we only use static/dependency-free content bundles--we aren't testing that Connect runs apps correctly here, we're just testing the API integration. |
Some of the failing content powers things like parameterized R Markdown / variant APIs, so while I like the second option better for simplicity's sake, it wouldn't be possible to keep the current suite of integration tests with just static content. For example: connectapi/tests/integrated/test-content.R Lines 431 to 455 in 4a4a810
Looks like the setup code for those tests is [here]( setup code). To generate manifests on the fly, we could use |
I guess we could do that in the test suite if we can run the tests using a version of R that is supported in all of the Connect images in our matrix. That should be doable? Or we need to expand the matrix definition to specify a tuple 2022.08.0 had R 4.1 rstudio/rstudio-docker-products@cfd6a87#diff-848e5afd960438beae1c9fdce3527cb65495623dd1a2eece90289719871d56a8R64 And so does 2025.04.0 from yesterday: https://github.com/rstudio/rstudio-docker-products/blob/main/connect/Dockerfile.ubuntu2204#L7 So maybe we target 4.1 with the integration tests, and drop any older Connect versions that don't have R 4.1? 2022.08 is beyond the support window anyway. |
Intent
Fixes #395
Approach
Add to the matrix