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

Add convenience methods to interact with container #69

Merged

Conversation

sbglasius
Copy link
Contributor

  • Exec in Container
  • Place files in container
  • Mount directories in container
  • Download files from container

* Exec in Container
* Place files in container
* Mount directories in container
* Download files from container
@sbglasius sbglasius requested a review from matrei November 1, 2024 09:59
@CLAassistant
Copy link

CLAassistant commented Nov 1, 2024

CLA assistant check
All committers have signed the CLA.

@sbglasius
Copy link
Contributor Author

Checked in a local version of grails-functional-tests (UploadControllerSpec):

void "Test file upload"() {
        when: "When go to an upload page"
        go "/upload/index"

        copyFileToContainer(Transferable.of("Test upload", 0777), "/test.txt")
        def form = $('#myForm')

        form.myFile = "/test.txt"
        $('#input1').click()

        then:"The file is uploaded"
        $('p').text() == 'Test upload'
    }

where without the convenience methods a cast is needed:

(webDriverContainer as WebDriverContainer).copyFile...

is needed.

Copy link
Contributor

@matrei matrei left a comment

Choose a reason for hiding this comment

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

Excellent!
There are some unused imports and some whitespace on line 101.

@matrei
Copy link
Contributor

matrei commented Nov 3, 2024

@jdaugherty Can you confirm that it works on MacOS with Testcontainers.exposeHostPorts(serverPort) before start()?

@matrei
Copy link
Contributor

matrei commented Nov 4, 2024

@sbglasius Would it be benefitial to expose the container via:

    BrowserWebDriverContainer getContainer() {
        return webDriverContainer
    }

And then you can access all methods on the container in the tests, eg: container.execInContainer(...) instead of creating methods in ContainerGebSpec that just delegates?

@sbglasius
Copy link
Contributor Author

Followed @matrei's suggestion, removed convenience methods and added getContainer() instead

@matrei
Copy link
Contributor

matrei commented Nov 4, 2024

Nice! Now there are some unused imports and a whitespace issue (Line 122) again.

If @jdaugherty, can confirm the MacOS fix, that should probably be in it's own PR or commit as it fixes a bug.

Otherwise, I'd say Squash and merge on the new feature!

@jdaugherty
Copy link
Contributor

I'm working on confirming the change on MacOS, but I noticed the README.md still contains references to GebConfig.groovy. Isn't this need removed now?

Copy link
Contributor

@jdaugherty jdaugherty left a comment

Choose a reason for hiding this comment

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

I can confirm the expose ports change fixes the MacOS issue.

@sbglasius
Copy link
Contributor Author

the reference to GebConfig.groovy is relevant if you use GebSpec instead of ContainerGebSpec

@sbglasius sbglasius merged commit 23a3044 into grails:5.0.x Nov 7, 2024
5 checks passed
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.

5 participants