Skip to content

Conversation

r0ss88
Copy link
Contributor

@r0ss88 r0ss88 commented Nov 22, 2023

Fix an issue when using docker context from WSL mounted path resolving as windows path (#993)

@ghost ghost added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Nov 22, 2023
@davidfowl
Copy link
Member

Hmm, we don't have any tests for this code path yet do we...

@r0ss88
Copy link
Contributor Author

r0ss88 commented Nov 22, 2023

No, I had noticed that :)

@mitchdenny
Copy link
Member

Testing this code is going to be a little awkward in terms of an end to end functional test - but perhaps it can be pulled out into an interna; helper method which we can call from Aspire.Hosting.Tests just to satisfy us that it covers all the various combinations that we could see.

@BrennanConroy
Copy link
Member

BrennanConroy commented Dec 5, 2023

Could you add a test like

public async Task VerifyDockerWithEntrypointWorks()

instead grabbing Container from KubernetesService and check the Spec.VolumeMounts property?

@joperezr joperezr added the community-contribution Indicates that the PR has been added by a community member label Dec 11, 2023
@BrennanConroy
Copy link
Member

@r0ss88 are you still interested in updating this?

@r0ss88
Copy link
Contributor Author

r0ss88 commented Jan 17, 2024

Apologies for the delayed response, I ended up working round this so no longer require this fix. I'd be happy to add the tests though to cover the change.

@r0ss88
Copy link
Contributor Author

r0ss88 commented Jan 22, 2024

Hey @BrennanConroy, I have added the required unit tests as requested.

@BrennanConroy BrennanConroy merged commit 0d85de5 into dotnet:main Jan 22, 2024
@BrennanConroy
Copy link
Member

Thank you @r0ss88 !

@github-actions github-actions bot locked and limited conversation to collaborators Apr 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants