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

Fix #5257: Add ErrorStreamMessage and StatusStreamMessage to ease mocking of pods/exec requests #5258

Conversation

Donnerbart
Copy link
Contributor

@Donnerbart Donnerbart commented Jun 15, 2023

Description

Fixes #5257

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift
  • I tested my code in my own tests

@Donnerbart Donnerbart force-pushed the improvement/5257-improve-mocking-capabilities branch 2 times, most recently from 7421bfd to ab14817 Compare June 16, 2023 08:12
@Donnerbart
Copy link
Contributor Author

The existing classes of kubernetes-server-mock have no JavaDoc and are not mentioned in the documentation. So I guess it's okay to tick that off the checklist.

The license and spotless checks are passing locally. I guess for the SonarCloud report someone needs to trigger the builds first.

Since this is not production code, I'm not sure what to do about the I tested my code in Kubernetes/OpenShift part. I've added the same code to our custom operator to write unit tests and there it works fine.

@Donnerbart Donnerbart marked this pull request as ready for review June 16, 2023 08:56
@Donnerbart
Copy link
Contributor Author

I think the failed SonarCloud analysis comes from a local ./mvnw clean install sonar:sonar -Psonar -DskipTests from my machine (no tests, no coverage). I'll wait until the CI runs it correctly.

@Donnerbart
Copy link
Contributor Author

Thanks for running the tests. I don't understand the Sonar report though. It claims that there is missing code coverage on the new classes, but they are used in PodTest. Do the tests have to be in the same module to count?

@Donnerbart Donnerbart force-pushed the improvement/5257-improve-mocking-capabilities branch 3 times, most recently from 54c811d to feda420 Compare June 19, 2023 08:44
@Donnerbart
Copy link
Contributor Author

I've checked all code smells with the SonarLint plugin locally and added unit tests in the same module for all touched and new classes. Let's hope SonarCloud is happy now :)

@Donnerbart Donnerbart force-pushed the improvement/5257-improve-mocking-capabilities branch from feda420 to 312a4fa Compare June 21, 2023 09:11
@Donnerbart
Copy link
Contributor Author

I've added the missing license headers to the new test files.

@Donnerbart Donnerbart force-pushed the improvement/5257-improve-mocking-capabilities branch 3 times, most recently from ff83e00 to bbe91ac Compare June 21, 2023 12:05
@Donnerbart
Copy link
Contributor Author

Donnerbart commented Jun 21, 2023

Fixed the spotless violation (sorry for the many pushes, it's a totally different tech stack for me).

@Donnerbart Donnerbart force-pushed the improvement/5257-improve-mocking-capabilities branch from bbe91ac to af781c8 Compare June 21, 2023 14:48
@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

100.0% 100.0% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@shawkins shawkins left a comment

Choose a reason for hiding this comment

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

These look much better for testing execs thank you

@Donnerbart Donnerbart force-pushed the improvement/5257-improve-mocking-capabilities branch from af781c8 to 847f893 Compare August 14, 2023 15:58
@Donnerbart
Copy link
Contributor Author

I've rebased the branch to solve the conflict in the CHANGELOG.md. No further changes or merge conflicts.

Any change to get this merged into the 6.9 release?

@Donnerbart Donnerbart changed the title Add ErrorStreamMessage and StatusStreamMessage to ease mocking of pods/exec requests Fix #5257: Add ErrorStreamMessage and StatusStreamMessage to ease mocking of pods/exec requests Aug 15, 2023
@Donnerbart
Copy link
Contributor Author

Donnerbart commented Aug 22, 2023

Looks like all E2E failed:

Error:  Failed to execute goal on project kubernetes-itests:
  Could not resolve dependencies for project io.fabric8:kubernetes-itests:jar:6.9-SNAPSHOT:
  The following artifacts could not be resolved:
  io.fabric8:kubernetes-junit-jupiter-autodetected:jar:6.9-SNAPSHOT,
  io.fabric8:openshift-client:jar:6.9-SNAPSHOT,
  io.fabric8:kubernetes-client-api:jar:tests:6.9-SNAPSHOT:
  Could not find artifact io.fabric8:kubernetes-junit-jupiter-autodetected:jar:6.9-SNAPSHOT

The PR is based on the latest main branch and I didn't change anything regarding the module setup. So I'm not sure if the root cause is in this PR or not.

@Donnerbart Donnerbart force-pushed the improvement/5257-improve-mocking-capabilities branch from 847f893 to 5aa8698 Compare August 29, 2023 14:34
@Donnerbart
Copy link
Contributor Author

Rebased with main branch and resolved the merge conflict in CHANGELOG.md. No further changes.

@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

100.0% 100.0% Coverage
0.0% 0.0% Duplication

warning The version of Java (11.0.20) you have used to run this analysis is deprecated and we will stop accepting it soon. Please update to at least Java 17.
Read more here

@manusa manusa added this to the 6.9.0 milestone Sep 6, 2023
@manusa manusa merged commit f58cdde into fabric8io:main Sep 6, 2023
@Donnerbart Donnerbart deleted the improvement/5257-improve-mocking-capabilities branch September 6, 2023 11:47
manusa added a commit that referenced this pull request Sep 6, 2023
manusa added a commit that referenced this pull request Sep 6, 2023
Ananya2001-an added a commit to Ananya2001-an/kubernetes-client that referenced this pull request Sep 7, 2023
chore:  add create namespace kubectl equivalent example

docs: add link to example in README

chore: separated creation of namespace and logging

fix: sonarcloud issue with printStackTrace

remove io.fabric8.kubernetes.model.annotation.PrinterColumn

Fix fabric8io#5257: Add ErrorStreamMessage and StatusStreamMessage to ease mocking of pods/exec requests (fabric8io#5258)

* Add ErrorStreamMessage and StatusStreamMessage to ease mocking of pods/exec requests

* Fix warnings in PodTest

refactor: follow-up on fabric8io#5258

Signed-off-by: Marc Nuri <[email protected]>

doc: invite users to report issues with a specific document

Signed-off-by: Marc Nuri <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve mocking capabilities
4 participants