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

feat: reimplement rserve route #813

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

Conversation

marikaris
Copy link
Collaborator

how to test:

  • explain here what to do to test this (or point to unit tests)

todo:

  • updated docs in case of new feature
  • added tests

@marikaris marikaris marked this pull request as ready for review November 14, 2024 07:06
@marikaris
Copy link
Collaborator Author

Sonar is failing, but coverage has been increased a lot. The added in code, is mostly old code that we're reintroducing. This code doesn't have many tests. Mostly because the complexity of tests that would have to be written, writing those tests would take up more time than having them would ever grant us.

Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
67.4% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@marikaris marikaris changed the title POC: reimplement rserve route feat: reimplement rserve route Nov 15, 2024
@marikaris marikaris requested a review from mswertz November 26, 2024 13:49
Copy link
Member

@mswertz mswertz left a comment

Choose a reason for hiding this comment

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

Code looks good. Would like a quick chat for final acceptance.

Is it possible to reuse exact tests between Rock and Rserve using a parameter to run all test twice using either or? Maybe even a totally outside parameter via gradle. Or is this maybe we what I see.

@marikaris
Copy link
Collaborator Author

Code looks good. Would like a quick chat for final acceptance.

Is it possible to reuse exact tests between Rock and Rserve using a parameter to run all test twice using either or? Maybe even a totally outside parameter via gradle. Or is this maybe we what I see.

I think the rock code lacks in the amount of tests it has. I have reintroduced the tests we had for rserve and added a couple new tests. I have tested both paths using our releasetest, using both profiles (rserve/rock) and that test succeeds both ways.

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.

2 participants