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

item(TCOMP-2743): Add missing httpclient validator unit test #903

Merged
merged 11 commits into from
Jun 28, 2024
Merged

item(TCOMP-2743): Add missing httpclient validator unit test #903

merged 11 commits into from
Jun 28, 2024

Conversation

yyin-talend
Copy link
Collaborator

@yyin-talend yyin-talend commented Jun 25, 2024 •
edited by acatoire
Loading

@yyin-talend yyin-talend requested a review from undx June 25, 2024 06:14
@sonar-eks sonar-eks

This comment has been minimized.

Sign in to view
undx
undx previously approved these changes Jun 25, 2024
View reviewed changes
Copy link
Member

@undx undx left a comment

Choose a reason for hiding this comment

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

LGTM
PLease look at sonar and fix issue.

@sonar-eks sonar-eks

This comment has been minimized.

Sign in to view
undx
undx previously approved these changes Jun 25, 2024
View reviewed changes
@sonar-eks sonar-eks

This comment has been minimized.

Sign in to view
acatoire
acatoire requested changes Jun 25, 2024
View reviewed changes
Copy link
Contributor

@acatoire acatoire left a comment

Choose a reason for hiding this comment

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

Small remark on validateClassMethodMissingAnnotation and WrongClient

...nt-tools/src/test/java/org/talend/sdk/component/tools/validator/HttpClientValidatorTest.java Outdated Show resolved Hide resolved
...nt-tools/src/test/java/org/talend/sdk/component/tools/validator/HttpClientValidatorTest.java Show resolved Hide resolved
@sonar-eks sonar-eks

This comment has been minimized.

Sign in to view
ypiel-talend
ypiel-talend reviewed Jun 25, 2024
View reviewed changes
Copy link
Contributor

@ypiel-talend ypiel-talend left a comment

Choose a reason for hiding this comment

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

some comments

component-tools/src/main/java/org/talend/sdk/component/tools/validator/HttpValidator.java Show resolved Hide resolved
...nt-tools/src/test/java/org/talend/sdk/component/tools/validator/HttpClientValidatorTest.java Outdated Show resolved Hide resolved
...nt-tools/src/test/java/org/talend/sdk/component/tools/validator/HttpClientValidatorTest.java Outdated Show resolved Hide resolved
...nt-tools/src/test/java/org/talend/sdk/component/tools/validator/HttpClientValidatorTest.java Outdated Show resolved Hide resolved
@sonar-eks sonar-eks

This comment has been minimized.

Sign in to view
4 similar comments
@sonar-eks sonar-eks

This comment has been minimized.

Sign in to view
@sonar-eks sonar-eks

This comment has been minimized.

Sign in to view
@sonar-eks sonar-eks

This comment has been minimized.

Sign in to view
@sonar-eks sonar-eks

This comment has been minimized.

Sign in to view
@acatoire
Copy link
Contributor

Could we add a test with:

    interface WrongClientNoMethodRequest extends HttpClient {

        // It misses @Request
        String queryA(String ok);

        // It misses @Request
        String queryB(String ok);
    }

acatoire
acatoire requested changes Jun 26, 2024
View reviewed changes
Copy link
Contributor

@acatoire acatoire left a comment

Choose a reason for hiding this comment

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

Small renaming comments

...nt-tools/src/test/java/org/talend/sdk/component/tools/validator/HttpClientValidatorTest.java Outdated Show resolved Hide resolved
...nt-tools/src/test/java/org/talend/sdk/component/tools/validator/HttpClientValidatorTest.java Outdated Show resolved Hide resolved
...nt-tools/src/test/java/org/talend/sdk/component/tools/validator/HttpClientValidatorTest.java Outdated Show resolved Hide resolved
...nt-tools/src/test/java/org/talend/sdk/component/tools/validator/HttpClientValidatorTest.java Outdated Show resolved Hide resolved
...nt-tools/src/test/java/org/talend/sdk/component/tools/validator/HttpClientValidatorTest.java Outdated
String queryB(String ok);
}

interface WithOtherAnnotationClient extends HttpClient {
Copy link
Contributor

Choose a reason for hiding this comment

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

Renaming for clear purpose

Suggested change
interface WithOtherAnnotationClient extends HttpClient {
interface WrongClientUsingOneWrongAnnotation

@sonar-eks sonar-eks

This comment has been minimized.

Sign in to view
@sonar-eks sonar-eks
Copy link

sonar-eks bot commented Jun 28, 2024

Passed

Analysis Details

0 Issues

  • Bug 0 Bugs
  • Vulnerability 0 Vulnerabilities
  • Code Smell 0 Code Smells

Coverage and Duplications

  • Coverage 100.00% Coverage (54.30% Estimated after merge)
  • Duplications 0.00% Duplicated Code (1.30% Estimated after merge)

Project ID: org.talend.sdk.component:component-runtime

View in SonarQube

@yyin-talend yyin-talend merged commit 5a36dfe into master Jun 28, 2024
6 checks passed
@yyin-talend yyin-talend deleted the yyin/TCOMP-2743-AddHttpValidatorJunit branch June 28, 2024 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ypiel-talend ypiel-talend ypiel-talend left review comments

@acatoire acatoire acatoire approved these changes

@undx undx undx left review comments

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

Successfully merging this pull request may close these issues.
4 participants
@yyin-talend @acatoire @undx @ypiel-talend