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

[test/plugin] Immigrate tests okhttp scenario #3905

Merged
merged 16 commits into from
Nov 25, 2019
Merged

[test/plugin] Immigrate tests okhttp scenario #3905

merged 16 commits into from
Nov 25, 2019

Conversation

wayilau
Copy link
Member

@wayilau wayilau commented Nov 20, 2019

Please answer these questions before submitting pull request

  • Why submit this pull request?
  • Bug fix
  • New feature provided
  • Improve performance
  • Tests cases

Immigrate tests okhttp scenario.

@wu-sheng
Copy link
Member

Your PR miss the apache 2.0 header. Please fix

@wu-sheng wu-sheng added agent Language agent related. plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor. test Test requirements about performance, feature or before release. labels Nov 20, 2019
@dmsolr
Copy link
Member

dmsolr commented Nov 20, 2019

Hi, We suggest and recommend to use maven archetype-plugin to generate a plugin test-case project. You can learn more information from how-to-use-the-archetype-to-create-a-test-case-project

@wayilau
Copy link
Member Author

wayilau commented Nov 20, 2019

Your PR miss the apache 2.0 header. Please fix

ok

@wayilau
Copy link
Member Author

wayilau commented Nov 20, 2019

Hi, We suggest and recommend to use maven archetype-plugin to generate a plugin test-case project. You can learn more information from how-to-use-the-archetype-to-create-a-test-case-project

i just copied. Is there any problem? If I must generated pro?

@wu-sheng
Copy link
Member

You still have HEADER issue, you should run the rat check locally.

@wu-sheng
Copy link
Member

Ref #3583

@wu-sheng
Copy link
Member

@dmsolr @arugal Test passed. Please review.

1 similar comment
@wu-sheng
Copy link
Member

@dmsolr @arugal Test passed. Please review.

@wayilau
Copy link
Member Author

wayilau commented Nov 22, 2019

@dmsolr . @arugal
Do you have time to help me review this PR? Thanks !

dmsolr
dmsolr reviewed Nov 22, 2019
View reviewed changes
Copy link
Member

@dmsolr dmsolr left a comment

Choose a reason for hiding this comment

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

please update the document

test/plugin/scenarios/okhttp-scenario/src/main/resources/log4j2.xml Outdated Show resolved Hide resolved
test/plugin/scenarios/okhttp-scenario/pom.xml Outdated Show resolved Hide resolved
...scenario/src/main/java/test/apache/skywalking/testcase/okhttp/controller/CaseController.java Outdated Show resolved Hide resolved
arugal
arugal reviewed Nov 22, 2019
View reviewed changes
Jenkinsfile-Agent-Test-2 Outdated Show resolved Hide resolved
@arugal
Copy link
Member

arugal commented Nov 22, 2019

Good for me. But you also need to update the-elapsed-time-list-of-plugins.

@arugal arugal changed the title Immigrate tests okhttp scenario [test/plugin] Immigrate tests okhttp scenario Nov 23, 2019
@wu-sheng
Copy link
Member

@wayilau Please follow the review, update the document.

@wu-sheng
Copy link
Member

wu-sheng commented Nov 23, 2019 •
edited
Loading

@wayilau As you are from CMSS Chinamobile, are you going to take part in tomorrow's Tencent TVP Suzhou event?

@wu-sheng
Copy link
Member

/run agent-plugin-test-4

@wayilau
Copy link
Member Author

wayilau commented Nov 24, 2019

@wayilau As you are from CMSS Chinamobile, are you going to take part in tomorrow's Tencent TVP Suzhou event?
@wu-sheng I didn't. I have to Take care of my children 。

@wayilau
Copy link
Member Author

wayilau commented Nov 24, 2019 •
edited
Loading

I've updated the doc. @arugal @wu-sheng @dmsolr

wu-sheng
wu-sheng reviewed Nov 24, 2019
View reviewed changes
docs/en/guides/Plugin-test.md
@@ -637,6 +637,7 @@ spring-tx 4.x+ | 10 | 555.00
spring 4.3.x-5.2.x | 54 | 1769.32
dubbo 2.5.x-2.6.x | 10 | 367.23
dubbo 2.7.x | 4 | 214.99
okhttp 3.0.x-3.14.x | 34 | 1030
Copy link
Member

Choose a reason for hiding this comment

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

You miss the total time update.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, i will modify.

arugal
arugal reviewed Nov 25, 2019
View reviewed changes
test/plugin/scenarios/okhttp-scenario/config/expectedData.yaml Outdated
instances:
- {okhttp-scenario: 1}
operationNames:
- okhttp-scenario: [/case/healthCheck, /case/okhttp-case, /case/receiveContext-0]
Copy link
Member

@arugal arugal Nov 25, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Hi, seems to lack /case/receiveContext-1?

Copy link
Member Author

Choose a reason for hiding this comment

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

I will check this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I have added it.

arugal
arugal approved these changes Nov 25, 2019
View reviewed changes
Copy link
Member

@arugal arugal left a comment

Choose a reason for hiding this comment

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

LGTM

@wu-sheng wu-sheng merged commit 8ad3bab into apache:master Nov 25, 2019
@wayilau wayilau deleted the dev2 branch November 25, 2019 05:31
@wu-sheng wu-sheng added this to the 6.6.0 milestone Dec 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wu-sheng wu-sheng wu-sheng approved these changes

@arugal arugal arugal approved these changes

@zhaoyuguang zhaoyuguang Awaiting requested review from zhaoyuguang

@dmsolr dmsolr Awaiting requested review from dmsolr

Assignees
No one assigned
Labels
agent Language agent related. plugin Plugin for agent or collector. Be used to extend the capabilities of default implementor. test Test requirements about performance, feature or before release.
Projects
None yet
Milestone
6.6.0
Development

Successfully merging this pull request may close these issues.
4 participants
@wayilau @wu-sheng @dmsolr @arugal