Adding public API test coverage for Aspire.Hosting.Elasticsearch #5119
Conversation
dotnet-issue-labeler
bot
added
the
area-integrations
dotnet-policy-service
bot
added
the
community-contribution
tests/Aspire.Hosting.Elasticsearch.Tests/ElasticsearchPublicApiTests.cs
Outdated
Show resolved
Hide resolved
tests/Aspire.Hosting.Elasticsearch.Tests/ElasticsearchPublicApiTests.cs
Outdated
Show resolved
Hide resolved
tests/Aspire.Hosting.Elasticsearch.Tests/ElasticsearchPublicApiTests.cs
Outdated
Show resolved
Hide resolved
|
@Zombach Small request for you or for whoever merges them, can you name them with a more meaningful title, something like "Adding public API test coverage for Aspire.Hosting.Elasticsearch". CA1062 is quite cryptic when you are not familiar with these PRs. |
Alirexaa
changed the title
…aspire-hosting-elasticsearch
| [Fact] | ||
| public void AddElasticsearchContainerShouldThrowWhenNameIsNull() | ||
| { | ||
| IDistributedApplicationBuilder builder = new DistributedApplicationBuilder([]); |
There was a problem hiding this comment.
As I found out. Better to use TestDistributedApplicationBuilder.Create();
There was a problem hiding this comment.
AFIK TestDistributedApplicationBuilder is suitable when starting the app. Here, it seems unnecessary.
There was a problem hiding this comment.
AFIK
TestDistributedApplicationBuilderis suitable when starting the app. Here, it seems unnecessary.
I wanted this more from the point of view that there would be a single code base for checking the public 'null'.
Although you may be right. View this at your own discretion. Well, or ask the guys what they think about this
…aspire-hosting-elasticsearch
…aspire-hosting-elasticsearch
|
@sebastienros, this PR is ready to merge. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Alirexaa
deleted the
validate-arguments-of-public-methods#aspire-hosting-elasticsearch
branch
Contributes to #2649 and #5047
Microsoft Reviewers: Open in CodeFlow