Postgres 4.0 by itowlson · Pull Request #44 · fermyon/conformance-tests

itowlson · 2025-07-15T03:27:40Z

This ports the Spin PostgreSQL integration test (which is not currently run because connection reset by peer) to the conformance tests framework. The motivation is the new PG types and so it jumps ahead to the spin:[email protected] interface, which is currently in review.

@rylev I am not sure how to test the test, or how to implement the postgres precondition. Also in terms of testing my Spin code with it, is the expectation that I would change Spin's conformance-tests reference to point to my branch, then... merge the new conformance test, update the reference in Spin, and then merge the Spin changes? I'd value any guidance you can offer on the recommended process here - thanks!

(Edit: Ah, I see there is already a Postgres service - I will noodle on that.)

Signed-off-by: itowlson <[email protected]>

itowlson · 2025-07-18T01:07:45Z

crates/conformance-tests/src/lib.rs

    let response = flate2::read::GzDecoder::new(response);
    let dir = std::env::temp_dir().join("conformance-tests");
+    if dir.exists() {
+        if let Some(err) = std::fs::remove_dir_all(&dir).err() {


I added this because I was finding that if I switched between local dev and canary conformance tests, the test I added in local dev remained in the untar directory, and caused the Spin tests to fail with a cryptic error. I can revert it if we have a strong preference for keeping the directory.

itowlson · 2025-07-18T01:10:38Z

crates/test-environment/src/services.rs

                    Box::new(DockerService::start(&service_def.name, image, &lock_dir)?)
                }
            };
+            // TODO: Postgres seems to show Ready before the DB is actually ready to accept connections.


I am not sure what to do about this - the PostgreSQL container becomes "healthy" quite early in its load cycle, but still takes a few seconds before it starts accepting connections. So without this the tests were failing with an "early eof". I guess this would be better handled by a more custom health test or something? Or a new service/test setting to specify a wait time? Open to recommendations.

I think we can add a health check to the postges.Dockerfile that looks like this:

HEALTHCHECK --interval=10s --timeout=5s --retries=5 \ CMD pg_isready -U postgres || exit 1

I believe the pg_isready command is available for us to use.

itowlson · 2025-07-18T01:15:26Z

crates/test-environment/src/services/docker.rs

 fn build_image(dockerfile_path: &Path, image_name: &String) -> anyhow::Result<()> {
-    let temp_dir = temp_dir::TempDir::new()
-        .context("failed to produce a temporary directory to run docker in")?;
+    let docker_context_dir = dockerfile_path.parent().unwrap();


Using a temp dir as the build context directory didn't work with the new artifacts used in the PostgreSQL Dockerfile. I was not sure if the use of a temp dir was critical - using the Dockerfile dir seemed to work and didn't leave any turds or anything. But I'm cautious because I'm changing something without understanding its motivation...!

I believe the idea with a temp directory was simply to not leave anything behind after running the test suite. If you can run the full suite without leaving anything behind, we can make this change.

I am curious what "didn't work" though?

Nit: use expect instead of unwrap just in case something really weird happens...

What didn't work was this line in the Postgres Dockerfile:

COPY ./postgres-init/ /docker-entrypoint-initdb.d/

The relative path to copy from was being taken as relative to the context directory rather than relative to the Dockerfile, resulting in the init directory not being found.

I could tree-copy the services directory to the temp dir if that would make you more comfortable? But I've not see any turds in the context directory - do you have a sense for when that might happen?

itowlson · 2025-08-11T20:31:44Z

The testee interface is now merged into Spin.

rylev

Looks good. I didn't test this myself locally, but if you're confident this runs, we should be fine to merge. The only thing I'd want to address before merging is the health check for postgres.

rylev · 2025-08-18T09:30:12Z

crates/conformance-tests/src/lib.rs

            .to_owned();
            let config = r#try!(std::fs::read_to_string(test_dir.join("test.json5"))
-                .context("failed to read test config"));
+                .context(format!("failed to read test config from {test_dir:?}")));


nit:

Suggested change

.context(format!("failed to read test config from {test_dir:?}")));

.with_context(|| format!("failed to read test config from {test_dir:?}")));

rylev · 2025-08-18T09:35:34Z

crates/test-environment/src/services.rs

                    Box::new(DockerService::start(&service_def.name, image, &lock_dir)?)
                }
            };
+            // TODO: Postgres seems to show Ready before the DB is actually ready to accept connections.


I think we can add a health check to the postges.Dockerfile that looks like this:

HEALTHCHECK --interval=10s --timeout=5s --retries=5 \ CMD pg_isready -U postgres || exit 1

I believe the pg_isready command is available for us to use.

rylev · 2025-08-18T09:38:03Z

crates/test-environment/src/services/docker.rs

 fn build_image(dockerfile_path: &Path, image_name: &String) -> anyhow::Result<()> {
-    let temp_dir = temp_dir::TempDir::new()
-        .context("failed to produce a temporary directory to run docker in")?;
+    let docker_context_dir = dockerfile_path.parent().unwrap();


I believe the idea with a temp directory was simply to not leave anything behind after running the test suite. If you can run the full suite without leaving anything behind, we can make this change.

I am curious what "didn't work" though?

Nit: use expect instead of unwrap just in case something really weird happens...

Signed-off-by: itowlson <[email protected]>

rylev

Looks good to me. @itowlson silly question: have you run these and made sure it works?

itowlson · 2025-08-19T19:41:33Z

@rylev I've run them locally via Spin and a fix-up of the archive path to point to a local tarball. All tests pass, and the Postgres test shows in the list of executed tests.

I'm not sure what more to do to verify that they'll work either in-tree in Spin or against hosts. I may well have missed something through having to re-point the internals; and I've not run the new Postgres health check as many times as I ran the old timer version. But this isn't "hit and hope" - believe me I saw I LOT of red along the way - and I don't know a way to gain more confidence short of merging and updating Spin to point to the new commit.

...I'm doomed aren't I

PostgreSQL test component

77b7dbd

Signed-off-by: itowlson <[email protected]>

itowlson force-pushed the postgres-4.0 branch 2 times, most recently from 1af2b95 to 5a75b9b Compare July 15, 2025 03:34

Postgres test manifest

dfb0c5d

Signed-off-by: itowlson <[email protected]>

itowlson force-pushed the postgres-4.0 branch 4 times, most recently from 02bd618 to 3e20fac Compare July 16, 2025 04:05

Initialise database in Postgres service

3c02a18

Signed-off-by: itowlson <[email protected]>

itowlson force-pushed the postgres-4.0 branch from 3e20fac to 3c02a18 Compare July 16, 2025 04:07

itowlson added 3 commits July 17, 2025 08:05

Fix old tests being kept around when switching between CT builds

c39abb4

Signed-off-by: itowlson <[email protected]>

Postgres tests pass. Just don't look at line 42 of services.rs

7d6a99f

Signed-off-by: itowlson <[email protected]>

Postgres range and array tests

fa529a6

Signed-off-by: itowlson <[email protected]>

itowlson commented Jul 18, 2025

View reviewed changes

rylev reviewed Aug 18, 2025

View reviewed changes

Fixed Postgres readiness, other minor fixes

6a5586c

Signed-off-by: itowlson <[email protected]>

itowlson requested a review from rylev August 18, 2025 21:02

rylev approved these changes Aug 19, 2025

View reviewed changes

itowlson merged commit 61f2799 into fermyon:main Aug 19, 2025
2 checks passed

	.context(format!("failed to read test config from {test_dir:?}")));
	.with_context(\|\| format!("failed to read test config from {test_dir:?}")));

Conversation

itowlson commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

itowlson commented Aug 11, 2025

Uh oh!

rylev 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!

rylev left a comment

Choose a reason for hiding this comment

Uh oh!

itowlson commented Aug 19, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

itowlson commented Jul 15, 2025 •

edited

Loading