Skip to content

Extract Aspire.Hosting.Kafka.Tests project #4910

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jul 16, 2024
Merged

Extract Aspire.Hosting.Kafka.Tests project #4910

merged 5 commits into from
Jul 16, 2024

Conversation

@eerhardt
Copy link
Member

@eerhardt eerhardt commented Jul 15, 2024 •
edited by dotnet-policy-service bot
Loading

Contributes to #3185
Contributes to #4294

cc @Alirexaa

Microsoft Reviewers: Open in CodeFlow

@ghost ghost added the area-integrations Issues pertaining to Aspire Integrations packages label Jul 15, 2024
eerhardt
eerhardt commented Jul 15, 2024
View reviewed changes
tests/Aspire.Hosting.Tests/ManifestGenerationTests.cs
}

[Fact]
public void EnsureAllKafkaManifestTypesHaveVersion0Suffix()
Copy link
Member Author

@eerhardt eerhardt Jul 15, 2024

Choose a reason for hiding this comment

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

I don't think this test adds value over the AddKafkaTests.VerifyManifest test. That test verifies it is a container.v0 resource.

Copy link
Member

@radical radical Jul 15, 2024

Choose a reason for hiding this comment

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

ok :)

@eerhardt eerhardt mentioned this pull request Jul 15, 2024
Closed
20 tasks
radical
radical reviewed Jul 15, 2024
View reviewed changes
@radical radical mentioned this pull request Jul 15, 2024
Merged
@radical radical mentioned this pull request Jul 16, 2024
Closed
sebastienros
sebastienros approved these changes Jul 16, 2024
View reviewed changes
Copy link
Member

@sebastienros sebastienros left a comment

Choose a reason for hiding this comment

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

Only thing that I notice is that you removed it from E2E tests, which is fine by me.

@eerhardt
Copy link
Member Author

eerhardt commented Jul 16, 2024

Only thing that I notice is that you removed it from E2E tests, which is fine by me.

Yes, I think this is my recommendation going forward. Now that we have these "Functional" tests individually for each resource, I don't think we need the EndToEnd test to cover every individual resource. It just isn't scalable.

Instead, I think we get the EndToEnd down to maybe ~5 core resources (a couple database, Redis, and maybe RabbitMQ) and the rest of these Hosting+Component tests are all in the functional tests. It makes the EndToEnd tests more maintainable.

@eerhardt eerhardt enabled auto-merge (squash) July 16, 2024 15:58
@eerhardt eerhardt disabled auto-merge July 16, 2024 15:58
@eerhardt eerhardt enabled auto-merge (squash) July 16, 2024 15:58
@eerhardt eerhardt merged commit 4374062 into dotnet:main Jul 16, 2024
@eerhardt eerhardt deleted the ExtractKafkaHostingTests branch July 16, 2024 16:08
@github-actions github-actions bot locked and limited conversation to collaborators Aug 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@radical radical radical left review comments

@sebastienros sebastienros sebastienros approved these changes

@mitchdenny mitchdenny Awaiting requested review from mitchdenny mitchdenny is a code owner

Assignees

No one assigned

Labels

area-integrations Issues pertaining to Aspire Integrations packages

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@eerhardt @radical @sebastienros