Skip to content
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

feat: added firebase module #2954

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: added firebase module #2954

wants to merge 3 commits into from

Conversation

xytis
Copy link

@xytis xytis commented Jan 30, 2025

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.

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

netlify bot commented Jan 30, 2025

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit aef1039
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/679ce0cb64f30900081176e9
😎 Deploy Preview https://deploy-preview-2954--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copy link
Collaborator

@stevenh stevenh left a comment

Choose a reason for hiding this comment

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

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 Outdated Show resolved Hide resolved
modules/firebase/examples_test.go Outdated Show resolved Hide resolved
modules/firebase/anchor_test.go Outdated Show resolved Hide resolved
// 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") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

question: Could you clarify why this requirement is needed?

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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).

modules/firebase/firebase.go Outdated Show resolved Hide resolved
modules/firebase/firebase_test.go Outdated Show resolved Hide resolved
modules/firebase/firebase_test.go Outdated Show resolved Hide resolved
modules/firebase/firebase_test.go Show resolved Hide resolved
return fmt.Sprintf("%s:%s", host, port.Port()), nil
}

func (c *FirebaseContainer) UIConnectionString(ctx context.Context) (string, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

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.

modules/firebase/ports.go Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants