feat: added firebase module #2954

xytis · 2025-01-30T18:46:12Z

What does this PR do?

This module allows running firebase emulator as a testcontainer.

It does currently depend on a forked docker image, residing under my ownership. There is no official firebase emulator docker image to my knowledge.

Why is it important?

We use firebase, thought someone else might be using firebase too.

netlify · 2025-01-30T18:46:30Z

✅ Deploy Preview for testcontainers-go ready!

Name	Link
🔨 Latest commit	`7c24032`
🔍 Latest deploy log	https://app.netlify.com/sites/testcontainers-go/deploys/67a08f75704fc60008d9bf87
😎 Deploy Preview	https://deploy-preview-2954--testcontainers-go.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

stevenh

Thanks for the PR, I've done a initial pass. The majority of comments center around bringing it inline with the latest template for modules in terms of style and correct behaviour e.g. handling non nil container with error.

modules/firebase/examples_test.go

modules/firebase/anchor_test.go

stevenh · 2025-01-30T19:06:53Z

modules/firebase/firebase.go

+// WithRoot sets the directory which is copied to the destination container as firebase root
+func WithRoot(rootPath string) testcontainers.CustomizeRequestOption {
+	return func(req *testcontainers.GenericContainerRequest) error {
+		if !strings.HasSuffix(rootPath, "/firebase") {


question: Could you clarify why this requirement is needed?

This has been an issue a year ago. Mounting did not correctly work at that time, and if the source directory did not match the destination directory, mounting would nest the directory instead of mapping it directly.

I see that the issue has been fixed, removing.

Ah, my bad. The issue still exits. I wrote a failing test to exhibit the issue.

Is there something incorrect with the way I copy over the configuration?

Also, I don't think that volume mount would work in this case, because firebase emulator creates a lot of trash data in the root catalog, which in some cases can even make testing flaky (especially if using DATA_DIRECTORY as a fixture).

I'll try to find some time to look. I had a some in progress work to fix up the copy behaviour which was odd.

If you have a test to exercise this that, would help alongside a description of the desired behaviour and what you're seeing instead.

modules/firebase/firebase.go

modules/firebase/firebase_test.go

stevenh · 2025-01-30T19:17:14Z

modules/firebase/ports.go

+	return fmt.Sprintf("%s:%s", host, port.Port()), nil
+}
+
+func (c *FirebaseContainer) UIConnectionString(ctx context.Context) (string, error) {


question: seems like lots of additional API surface area instead of just exposing ConnectionString, thoughts?

If not then we'd need comments for all the methods.

This one is a bit more tricky.

I would love to change all of these methods, and figure out a better way to configure the launched container.

Currently I ship a catalog which contains static predefined configuration for the emulator. These methods happen to match the port numbers defined in the said file.

But, in the tests that we use, we provide a completely different setup (because we don't want to boot the full emulation suite), and we have to match the port numbers in our config so that all of this works.

So a question from my side would be:
Is there a way to generate file based config during container setup? To my knowledge firebase emulator does not allow passing those values as environment variables.

I think, ideal use example would be this:

firebaseContainer, err := firebase.Run(ctx, "ghcr.io/u-health/docker-firebase-emulator:13.29.2", firebase.WithUI(), firebase.WithFirestore(), firebase.WithCache(), )

And then, helper methods XXXConnectionString would actually return meaningful errors if the emulator part was not launched.

Is there a way to generate file based config during container setup?

@xytis you can take a look at the redpanda module, where there is a Go template the module is using to build the configuration file.

After some testing on my side, I remembered why I shipped the config as a directory in the first place.

Default (and probably expected) use case for firebase projects, is to run firebase init and then use the scaffolded configuration to interact with the dependencies.

In the following commit, I've changed the behaviour of this module. It now attempts to understand the emulator configuration of your project and then boots the containerised emulator with that info.

At least in my use case, that makes sense, and when I launch the tests, I use the same project-wide config for all of them.

stevenh · 2025-01-30T19:17:32Z

modules/firebase/ports.go

+func (c *FirebaseContainer) connectionString(ctx context.Context, portName nat.Port) (string, error) {
+	host, err := c.Host(ctx)
+	if err != nil {
+		return "", err


suggestion: wrap the error so the user knows the cause, more below.

stevenh

Thanks for the updates

.github/workflows/ci.yml

.vscode/.testcontainers-go.code-workspace

modules/firebase/config.go

modules/firebase/firebase_test.go

stevenh · 2025-01-31T17:15:36Z

modules/firebase/firebase_test.go

+	assert.Equal(t, "bar", out["foo"])
+}
+
+func TestFirebaseBadDirectory(t *testing.T) {


question: Is this issue you were referencing as in you are expecting:
./failure to be copied to /srv/firebase and instead if ends up as /srv/failure?

Yes, exactly.

And if I attempt to specify the destination path as /srv/firebase/ I get either /srv/firebase/firebase or /srv/firebase/failure, neither of which work for my use case.

I could attempt to modify the container launch script to accommodate for this, but firebase executable expects the configuration to be in the launch dir. That means, I would have to stray away from default execution method, and other containers would be less likely to work with this adapter.

Thanks, I was looking for a good example of this in real world.

modules/firebase/firebase_test.go

sonar-project.properties

mdelapenya

I added a few minor comments, we are in the good way.

Thanks for your contribution

mdelapenya · 2025-01-31T09:05:18Z

docs/modules/firebase.md

+
+### Container Methods
+
+The Firebase container exposes the following methods:


docs: we probably need to append here the new methods the container expose

mdelapenya · 2025-01-31T09:06:35Z

docs/modules/firebase.md

+#### Image
+
+If you need to set a different Firebase Docker image, you can use `testcontainers.WithImage` with a valid Docker image
+for Firebase. E.g. `testcontainers.WithImage("ghcr.io/thoughtgears/docker-firebase-emulator:13.6.0")`.


bug? this image does not match the one used in the module. See https://github.com/testcontainers/testcontainers-go/pull/2954/files#diff-c3cdb8ddcc6a1ed362f8710b6f57133757d0bec63d0c665a27ee2fca6bcd617cR20

mdelapenya · 2025-02-03T12:23:06Z

.github/workflows/ci.yml

@@ -94,7 +94,7 @@ jobs:
      matrix:
        go-version: [1.22.x, 1.x]
        platform: [ubuntu-latest]
-        module: [artemis, azurite, cassandra, chroma, clickhouse, cockroachdb, compose, consul, couchbase, databend, dolt, dynamodb, elasticsearch, etcd, gcloud, grafana-lgtm, inbucket, influxdb, k3s, k6, kafka, localstack, mariadb, meilisearch, milvus, minio, mockserver, mongodb, mssql, mysql, nats, neo4j, ollama, openfga, openldap, opensearch, postgres, pulsar, qdrant, rabbitmq, redis, redpanda, registry, surrealdb, valkey, vault, vearch, weaviate, yugabytedb]
+        module: [artemis, azurite, cassandra, chroma, clickhouse, cockroachdb, compose, consul, couchbase, databend, dolt, dynamodb, elasticsearch, etcd, firebase, gcloud, grafana-lgtm, inbucket, influxdb, k3s, k6, kafka, localstack, mariadb, meilisearch, milvus, minio, mockserver, mongodb, mssql, mysql, nats, neo4j, ollama, openfga, openldap, opensearch, postgres, pulsar, qdrant, rabbitmq, redis, redpanda, registry, surrealdb, valkey, vault, vearch, weaviate, yugabytedb]


We merged pinecone very recently. Could you please update this line, as it will get a conflict here?

mdelapenya · 2025-02-03T12:24:22Z

docs/modules/firebase.md

+
+!!!info
+    The `RunContainer(ctx, opts...)` function is deprecated and will be removed in the next major release of _Testcontainers for Go_.


Let's remove this block. It comes from the template, we need to update it on our end.

Suggested change

!!!info

The `RunContainer(ctx, opts...)` function is deprecated and will be removed in the next major release of _Testcontainers for Go_.

mdelapenya · 2025-02-03T12:26:14Z

modules/firebase/failure/.firebaserc

@@ -0,0 +1,5 @@
+{


suggestion: I'd move this dir into testdata so that it's clear it's used for testing purposes.

mdelapenya · 2025-02-03T12:27:32Z

modules/firebase/firebase.go

+
+// WithData names the data directory (by default under firebase root), can be used as a way of setting up fixtures.
+// Usage of absolute path will imply that the user knows how to mount external directory into the container.
+func WithData(dataPath string) testcontainers.CustomizeRequestOption {


question: if this method is not used elsewhere, let's remove it, or instead let's add a test using it

mdelapenya · 2025-02-03T12:28:47Z

docs/modules/firebase.md

+E.g. `Run(context.Background(), "ghcr.io/u-health/docker-firebase-emulator:13.29.2")`.
+
+{% include "../features/common_functional_options.md" %}
+


docsd: we need to add here all the functional options for the module.

feat: added firebase module

29b1bba

xytis requested a review from a team as a code owner January 30, 2025 18:46

chore: bump default container version

6a88123

stevenh requested changes Jan 30, 2025

View reviewed changes

stevenh requested changes Jan 31, 2025

View reviewed changes

fix: rewrote the module based on comments

4105b0a

xytis force-pushed the main branch from aef1039 to 4105b0a Compare February 2, 2025 08:40

fix: another pass at comments

cc2bfa4

xytis force-pushed the main branch from 7404470 to cc2bfa4 Compare February 2, 2025 09:37

fix: ports are uint

7c24032

mdelapenya reviewed Feb 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: added firebase module #2954

feat: added firebase module #2954

xytis commented Jan 30, 2025

netlify bot commented Jan 30, 2025 •

edited

Loading

stevenh left a comment

stevenh Jan 30, 2025

xytis Jan 31, 2025

xytis Jan 31, 2025

stevenh Jan 31, 2025

stevenh Jan 30, 2025

xytis Jan 31, 2025

mdelapenya Jan 31, 2025

xytis Jan 31, 2025

stevenh Jan 30, 2025

stevenh left a comment

stevenh Jan 31, 2025

xytis Feb 2, 2025

stevenh Feb 2, 2025

mdelapenya left a comment

mdelapenya Jan 31, 2025

mdelapenya Jan 31, 2025

mdelapenya Feb 3, 2025

mdelapenya Feb 3, 2025

mdelapenya Feb 3, 2025

mdelapenya Feb 3, 2025

mdelapenya Feb 3, 2025


		### Container Methods

		The Firebase container exposes the following methods:


		!!!info
		The `RunContainer(ctx, opts...)` function is deprecated and will be removed in the next major release of _Testcontainers for Go_.

		E.g. `Run(context.Background(), "ghcr.io/u-health/docker-firebase-emulator:13.29.2")`.

		{% include "../features/common_functional_options.md" %}

feat: added firebase module #2954

Are you sure you want to change the base?

feat: added firebase module #2954

Conversation

xytis commented Jan 30, 2025

What does this PR do?

Why is it important?

netlify bot commented Jan 30, 2025 • edited Loading

✅ Deploy Preview for testcontainers-go ready!

stevenh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevenh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdelapenya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

netlify bot commented Jan 30, 2025 •

edited

Loading