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.

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

HPCC4J-590 Add WsResources test cases #699

Merged
merged 1 commit into from
Apr 4, 2024
Merged

HPCC4J-590 Add WsResources test cases #699

merged 1 commit into from
Apr 4, 2024

Conversation

rpastrana
Copy link
Member

@rpastrana rpastrana commented Mar 27, 2024 •
edited
Loading

  • Adds test cases to account for all wsresources methods
  • Adds logic to create HPCCQueueType based on string representation
  • Adds all available methods to HPCCWsResourcesClient

Type of change:

  • This change is a bug fix (non-breaking change which fixes an issue).
  • This change is a new feature (non-breaking change which adds functionality).
  • This change is a breaking change (fix or feature that will cause existing behavior to change).

Checklist:

  • I have created a corresponding JIRA ticket for this submission
  • My code follows the code style of this project.
    • I have applied the Eclipse code-format template provided.
  • My change requires a change to the documentation.
    • I have updated the documentation accordingly, or...
    • I have created a JIRA ticket to update the documentation.
    • Any new interfaces or exported functions are appropriately commented.
  • I have read the HPCC Systems CONTRIBUTORS document (https://github.com/hpcc-systems/HPCC-Platform/wiki/Guide-for-contributors).
  • The change has been fully tested:
    • This change does not cause any existing JUnits to fail.
    • I have include JUnit coverage to test this change
    • I have performed system test and covered possible regressions and side effects.
  • I have given due consideration to all of the following potential concerns:
    • Scalability
    • Performance
    • Security
    • Thread-safety
    • Premature optimization
    • This change fixes the problem, not just the symptom

Testing:

@rpastrana rpastrana requested a review from jpmcmu March 27, 2024 15:58
jpmcmu
jpmcmu approved these changes Mar 28, 2024
View reviewed changes
Copy link
Contributor

@jpmcmu jpmcmu left a comment

Choose a reason for hiding this comment

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

@rpastrana Looks good, only issue is with the manual edits to the generated code, but not much we can do at the moment about that issue.

...t/src/main/java/org/hpccsystems/ws/client/wrappers/gen/wsresources/HPCCQueueTypeWrapper.java
@@ -54,9 +56,16 @@ public String toString()
}
public org.hpccsystems.ws.client.gen.axis2.wsresources.latest.HPCCQueueType getRaw()
Copy link
Contributor

Choose a reason for hiding this comment

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

@rpastrana As we discussed offline this will likely be headache down the line, but there doesn't seem like much we could do here at the moment. In the future it might be worthwhile having the wrappers be auto-generated and subclass them to override behavior. That would require some changes to other parts of the code that construct the wrappers, but would could probably have a "factory" method on the wrapper that can be overridden as well so the calling code doesn't have to know that the correct subclass to construct.

Copy link
Member

Choose a reason for hiding this comment

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

I may not understand what the 'raw' means. Why not change the name to getHPCCQueueType()?

Copy link
Member Author

Choose a reason for hiding this comment

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

@wangkx this is actually an accepted convention for our generated wrapper code, the wrapper exposes the underlying raw types via the getraw methods

Copy link
Member

Choose a reason for hiding this comment

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

OK

@rpastrana rpastrana force-pushed the HPCC4J-590-CreateWsResourcesJunits branch from 63369bc to 134b1e2 Compare April 1, 2024 19:29
@rpastrana rpastrana requested a review from jpmcmu April 1, 2024 23:47
@rpastrana
XXX-HPCC4J Add WsResources test cases
891e1b3 
- Adds test cases to account for all wsresources methods
- Adds logic to create HPCCQueueType based on string representation
- Adds all available methods to HPCCWsResourcesClient

Signed-off-by: Rodrigo Pastrana <[email protected]>
@rpastrana rpastrana force-pushed the HPCC4J-590-CreateWsResourcesJunits branch from 134b1e2 to 891e1b3 Compare April 1, 2024 23:52
jpmcmu
jpmcmu approved these changes Apr 2, 2024
View reviewed changes
Copy link
Contributor

@jpmcmu jpmcmu left a comment

Choose a reason for hiding this comment

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

@rpastrana looks good to me

@rpastrana rpastrana requested a review from wangkx April 3, 2024 13:59
@rpastrana
Copy link
Member Author

@wangkx please take a look at the test cases added here for wsresources.

wangkx
wangkx reviewed Apr 4, 2024
View reviewed changes
Copy link
Member

@wangkx wangkx left a comment

Choose a reason for hiding this comment

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

@rpastrana 2 minor comments

...t/src/main/java/org/hpccsystems/ws/client/wrappers/gen/wsresources/HPCCQueueTypeWrapper.java
@@ -54,9 +56,16 @@ public String toString()
}
public org.hpccsystems.ws.client.gen.axis2.wsresources.latest.HPCCQueueType getRaw()
Copy link
Member

Choose a reason for hiding this comment

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

I may not understand what the 'raw' means. Why not change the name to getHPCCQueueType()?

wsclient/src/test/java/org/hpccsystems/ws/client/WSResroucesClientTest.java Show resolved Hide resolved
@wangkx wangkx self-requested a review April 4, 2024 13:28
wangkx
wangkx approved these changes Apr 4, 2024
View reviewed changes
Copy link
Member

@wangkx wangkx left a comment

Choose a reason for hiding this comment

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

@rpastrana looks fine

@rpastrana rpastrana merged commit 9e974c9 into hpcc-systems:candidate-9.6.x Apr 4, 2024
4 of 6 checks passed
drealeed pushed a commit that referenced this pull request May 13, 2024
@rpastrana @drealeed
XXX-HPCC4J Add WsResources test cases (#699)
b14461f 
- Adds test cases to account for all wsresources methods
- Adds logic to create HPCCQueueType based on string representation
- Adds all available methods to HPCCWsResourcesClient

Signed-off-by: Rodrigo Pastrana <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpmcmu jpmcmu jpmcmu approved these changes

@wangkx wangkx wangkx approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rpastrana @wangkx @jpmcmu