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

Drop more overlapping tests (3) #1846

Merged
merged 8 commits into from
Jan 20, 2025

Conversation

wind57
Copy link
Contributor

@wind57 wind57 commented Jan 19, 2025

No description provided.

Signed-off-by: wind57 <[email protected]>
Signed-off-by: wind57 <[email protected]>
@wind57 wind57 changed the title Drop more overlaping tests Drop more overlapping tests (3) Jan 19, 2025
Signed-off-by: wind57 <[email protected]>
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the reason to drop this is this:

  • we have a test that uses spring.cloud.discovery.reactive.enabled=false
  • we also have a test that does the reverse : spring.cloud.discovery.reactive.enabled=true

the test that is dropped tests the "default" configuration, meaning it does not have the above property at all, which in turn means it defaults to spring.cloud.discovery.reactive.enabled=false. But such a case is also tested in a plain test, thus not really needed here.

Signed-off-by: wind57 <[email protected]>
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we already have a case where we have this exact test, but in an "extended" form. Here we use the filter as:

"spring.cloud.kubernetes.discovery.filter=#root.metadata.namespace matches 'a-uat$'"

in the other one, we use it as :

"spring.cloud.kubernetes.discovery.filter=#root.metadata.namespace matches '^.*uat$'"

so these are overlapping.

/**
* @author wind57
*/
class Fabric8DiscoveryPodMetadataIT extends Fabric8DiscoveryBase {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this was nor dropped, but inlined in a different test

@wind57 wind57 marked this pull request as ready for review January 20, 2025 16:28
Signed-off-by: wind57 <[email protected]>
@wind57
Copy link
Contributor Author

wind57 commented Jan 20, 2025

@ryanjbaxter this one is ready too. Same idea with overlapping tests... Thank you

@ryanjbaxter ryanjbaxter added this to the 3.1.6 milestone Jan 20, 2025
@ryanjbaxter ryanjbaxter merged commit c44fbeb into spring-cloud:3.1.x Jan 20, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Status: Done
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants