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

bug: Add missing import to test templates #2828

Closed
wants to merge 8 commits into from

Conversation

ballcoach12
Copy link

What does this PR do?

This PR adds a missing import for "github.com/testcontainers/testcontainers-go" to the examples_test.go.tmpl and module_test.go.tmpl files.

Why is it important?

Without this import, generating a new module will return an error code because the required module is not imported in the test files.

Related issues

How to test this PR

Generate a new module according to the instructions listed here. Verify that the list of imports contains "github.com/testcontainers/testcontainers-go" in the example test file and module test file.

@ballcoach12 ballcoach12 requested a review from a team as a code owner October 16, 2024 15:22
Copy link

netlify bot commented Oct 16, 2024

Deploy Preview for testcontainers-go failed.

Name Link
🔨 Latest commit 7e241d6
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/67111589676f400008379337

Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

Nit: the lint will complain if we do not add these blank lines separating imports

Other than that, LGTM

modulegen/_template/examples_test.go.tmpl Show resolved Hide resolved
modulegen/_template/module_test.go.tmpl Show resolved Hide resolved
@ballcoach12 ballcoach12 changed the title Add missing import to test templates bug: Add missing import to test templates Oct 16, 2024
@mdelapenya mdelapenya added the bug An issue with the library label Oct 16, 2024
@mdelapenya mdelapenya self-assigned this Oct 16, 2024
@mdelapenya
Copy link
Member

@ballcoach12 just to verify this is working, can you paste the output of generating a random module from your workspace? 🙏

@ballcoach12
Copy link
Author

I'm getting an error back, but the generated code appears to be correct.

Error: >> error synchronizing the dependencies: exit status 2
Usage:
  modulegen new module [flags]

Flags:
  -h, --help           help for module
  -i, --image string   Fully-qualified name of the Docker image to be used by the module
  -n, --name string    Name of the module. Only alphabetical characters are allowed.
  -t, --title string   (Optional) Title of the module name, used to override the name in the case of mixed casing (Mongodb -> MongoDB). Use camel-case when needed. Only alphabetical characters are allowed.

>> error synchronizing the dependencies: exit status 2
exit status 1

@mdelapenya
Copy link
Member

So it means there is something else in the formatting 🤔 Could you please take a deep look into the generated files and the templates ?

@ballcoach12
Copy link
Author

So just debugging the make lint command run that is called from exec.go, I captured the output and it looks like this:

out: go install github.com/golangci/golangci-lint/cmd/[email protected]
golangci-lint run --out-format=colored-line-number --path-prefix=. --verbose -c /.golangci.yml --fix
 err: process_begin: CreateProcess(NULL, dirname g:/BITBUCKET/testcontainers-go-github/testcontainers-go/commons-test.mk, ...) failed.
../../commons-test.mk:1: pipe: No error
level=info msg="golangci-lint has version v1.61.0 built with go1.22.2 from (unknown, modified: ?, mod sum: \"h1:VvbOLaRVWmyxCnUIMTbf1kDsaJbTzH20FAMXTAlQGu8=\") on (unknown)"
Error: can't load config: can't read viper config: open /.golangci.yml: The system cannot find the file specified.
Failed executing command with error: can't load config: can't read viper config: open /.golangci.yml: The system cannot find the file specified.
make: *** [../../commons-test.mk:33: lint] Error 3

So maybe a relative path is incorrect?

@mdelapenya
Copy link
Member

I think 69594b8 should be reverted, as it does not belong to this branch, shouldn't it?

@ballcoach12
Copy link
Author

ballcoach12 commented Oct 17, 2024

Yes. My mistake there. Would it be better to delete this PR while I track down and fix the error that is occurring on the module generation? It feels like this PR is drifting a little.

Remove proprietary modules
@mdelapenya
Copy link
Member

@ballcoach12 thanks for this contribution, but I think #2831 resolved it in a cleaner manner. In any case, thanks for your time and dedication for improving testcontainers-go! 💪

@mdelapenya mdelapenya closed this Oct 18, 2024
@ballcoach12
Copy link
Author

No worries. Sorry I couldn't be of more help.

@mdelapenya
Copy link
Member

@ballcoach12 there are and there will be more issues/bugs to work on, so I encourage you to help the community in making progress with your work. Just let us know if we can be of any help

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue with the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants