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

Make download*() methods work in ContainerGebSpec #74

Merged
merged 11 commits into from
Nov 15, 2024

Conversation

matrei
Copy link
Contributor

@matrei matrei commented Nov 11, 2024

Resolves gh-72

There must be a more elegant way to solve this, but this seems to work.

Resolves gh-72
…inerGebSpec`

Allows specifying `hostName` directly in `ContainerGebSpec` specs, e.g., `String hostName = 'my.test.app'`, to streamline test setup and improve readability.
Replaced direct field access with state getters to improve readability and encapsulation. This change makes it clearer how state is accessed and managed within the class.
…ation

- Converted `ContainerGebSpec` to an abstract class to ensure it is only used as a base class for other specs.
- This change enforces intended usage patterns and prevents accidental instantiation of `ContainerGebSpec`.
…dSupport`

Renamed `ContainerAwareDownloader` to `ContainerAwareDownloadSupport` to better reflect that it's a `DownloadSupport` implementation.
Copy link

@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 didn’t see another way to do this. Great Job and thanks for fixing this!

@matrei
Copy link
Contributor Author

matrei commented Nov 15, 2024

apache/groovy-geb#204

@matrei
Copy link
Contributor Author

matrei commented Nov 15, 2024

@sbglasius Do you want to take a look before I merge this?

Copy link
Contributor

@sbglasius sbglasius left a comment

Choose a reason for hiding this comment

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

You can fix my two suggestions if you want.

With this commit `ContainerAwareDownloadSupport` uses an inner class for delegating
to `DefaultDownloadSupport` instead of duplicating all its functionality.

As all `download*()` methods eventually invokes `download(Map options)` it is enough to
override this method, and changing the `base` option in the map to use the proper url.
@matrei matrei changed the title fix: download*() methods work Make download*() methods work in ContainerGebSpec Nov 15, 2024
@matrei matrei merged commit 0175015 into 5.0.x Nov 15, 2024
5 checks passed
@matrei matrei deleted the matrei/fix-download-methods branch November 15, 2024 11:52
@sbglasius
Copy link
Contributor

sbglasius commented Nov 15, 2024

@matrei Well done with the @Delegate instead of duplicating code.

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.

ContainerGebSpec.download* methods fails with UnknownHostException
3 participants