Skip to content

feat: integration test for cross reference dependent resource #1203

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
May 11, 2022
Merged

feat: integration test for cross reference dependent resource #1203

merged 5 commits into from
May 11, 2022

Conversation

csviri
Copy link
Collaborator

@csviri csviri commented May 9, 2022

No description provided.

@csviri csviri self-assigned this May 9, 2022
@csviri csviri linked an issue May 9, 2022 that may be closed by this pull request
AbstractDependentResource should be able to pass his turn #1105
Closed
@csviri csviri marked this pull request as ready for review May 10, 2022 10:00
@csviri
Copy link
Collaborator Author

csviri commented May 10, 2022

@metacosm first I wanted this just to reproduce the issue, but probably won't hurt to merge it to main.

metacosm
metacosm reviewed May 10, 2022
View reviewed changes
Copy link
Collaborator

@metacosm metacosm left a comment

Choose a reason for hiding this comment

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

I'm ambivalent on whether to add this test or not, actually. We could retool it for performance testing or maybe only run it in some cases like once a day or something so that we don't always run rather intensive tests.

@csviri @metacosm
Update operator-framework/src/test/java/io/javaoperatorsdk/operator/s…
8a7e2d6 
…ample/dependentresourcecrossref/DependentResourceCrossRefResource.java

Co-authored-by: Chris Laprun <[email protected]>
@csviri
Copy link
Collaborator Author

csviri commented May 10, 2022

I'm ambivalent on whether to add this test or not, actually. We could retool it for performance testing or maybe only run it in some cases like once a day or something so that we don't always run rather intensive tests.

Maybe we could have it less intensive. Like not 500 but just 50.

( TBH not sure if that causes lot's of overhead. Actually it runs very fast already and k8s api should handle such load without any issues. But I probably would merge it with reducted number).

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

metacosm
metacosm approved these changes May 11, 2022
View reviewed changes
Copy link
Collaborator

@metacosm metacosm left a comment

Choose a reason for hiding this comment

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

LGTM

@csviri csviri merged commit 9203295 into main May 11, 2022
@csviri csviri deleted the concurrency-issue-dependents branch May 11, 2022 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@metacosm metacosm metacosm approved these changes

Assignees

@csviri csviri

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AbstractDependentResource should be able to pass his turn
2 participants
@csviri @metacosm