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

Immigrate test mysql scenario. #3948

Merged
merged 15 commits into from
Nov 30, 2019
Merged

Immigrate test mysql scenario. #3948

merged 15 commits into from
Nov 30, 2019

Conversation

wayilau
Copy link
Member

@wayilau wayilau commented Nov 27, 2019

Please answer these questions before submitting pull request

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

@wu-sheng wu-sheng added feature New feature test Test requirements about performance, feature or before release. labels Nov 27, 2019
@wu-sheng wu-sheng added this to the 6.6.0 milestone Nov 27, 2019
@wu-sheng
Copy link
Member

/run e2e

@wu-sheng
Copy link
Member

@wayilau Send a mail to your cmss mailbox, please check.

@wayilau
Copy link
Member Author

wayilau commented Nov 27, 2019

@wayilau Send a mail to your cmss mailbox, please check.

@wu-sheng I received message you just wrote.

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.

Make it works and then update the elapsed time of the document. :)

instances:
- {mysql-scenario: 1}
operationNames:
- mysql-scenario: [/mysql-scenario/case/mysql]
Copy link
Member

Choose a reason for hiding this comment

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

Are you missing some operatioName? For example, /JDBI/statement/execute.

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 copy it from original source.

Jenkinsfile-Agent-Test-3 Outdated Show resolved Hide resolved
@arugal
Copy link
Member

arugal commented Nov 27, 2019

Ref #3583

@arugal
Copy link
Member

arugal commented Nov 27, 2019

Hi, I have a question, what do we not use JVM-container

@wayilau
Copy link
Member Author

wayilau commented Nov 28, 2019

Hi, I have a question, what do we not use JVM-container

original tests cases use tomcat, so i follow. @arugal

@wu-sheng
Copy link
Member

Jenkins jobs have been disabled, please add your tasks to GitHub action file, and fix the conflict, remove the Jenkins file. https://github.com/apache/skywalking/blob/master/.github/workflows/plugins-test.yaml

@wayilau
Copy link
Member Author

wayilau commented Nov 28, 2019

Jenkins jobs have been disabled, please add your tasks to GitHub action file, and fix the conflict, remove the Jenkins file. https://github.com/apache/skywalking/blob/master/.github/workflows/plugins-test.yaml

ok.

@wayilau
Copy link
Member Author

wayilau commented Nov 28, 2019

Hi, I have a question, what do we not use JVM-container

original tests cases use tomcat, so i follow. @arugal

As I have moved to JVM container. Beacause tomcat met some problems.

@dmsolr
Copy link
Member

dmsolr commented Nov 29, 2019

It is very sad to tell you that you need to update the Action configuration (resolve conflicts) because those are updated after #3956 merged. Maybe you need to move it to another group which is faster.
And the test failed (Tomcat-container doesn't need to pack it as a ZIP package. )

@wayilau
Copy link
Member Author

wayilau commented Nov 29, 2019

It is very sad to tell you that you need to update the Action configuration (resolve conflicts) because those are updated after #3956 merged. Maybe you need to move it to another group which is faster.
And the test failed (Tomcat-container doesn't need to pack it as a ZIP package. )

ok, I am using jVM container. May be i lost some files. i will check then submit again.

@wu-sheng
Copy link
Member

Jenkins is not there any more. We are working out with Apache why those are still there.

@wu-sheng
Copy link
Member

You should add MySQL As a separated group in that test case, named As MySQL case like others.

@wayilau
Copy link
Member Author

wayilau commented Nov 30, 2019

You should add MySQL As a separated group in that test case, named As MySQL case like others.

ok

@codecov-io
Copy link

codecov-io commented Nov 30, 2019

Codecov Report

Merging #3948 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #3948   +/-   ##
======================================
  Coverage    27.9%   27.9%           
======================================
  Files        1122    1122           
  Lines       24341   24341           
  Branches     3528    3528           
======================================
  Hits         6792    6792           
  Misses      16949   16949           
  Partials      600     600

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bdc0609...c3b30d4. Read the comment docs.

@wu-sheng
Copy link
Member

What is PluginsTest / .github/workflows/plugins-test.yaml (pull_request)?

run: bash test/plugin/run.sh kafka-scenario

Mysql:
Copy link
Member

Choose a reason for hiding this comment

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

This is not a correct format, This is a YAML.

Copy link
Member

Choose a reason for hiding this comment

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

The indentation is not correct IMO, syntax looks good

@dmsolr
Copy link
Member

dmsolr commented Nov 30, 2019

It still fails. Please recheck.

@wayilau
Copy link
Member Author

wayilau commented Nov 30, 2019

Log tells me that docker startup failed. I don't know why. Could you help me check ? @dmsolr

@wu-sheng
Copy link
Member

@wayilau I think it was caused by the wrong YAML format. Has been fixed. Now, you need to make your tests passed.

@wayilau
Copy link
Member Author

wayilau commented Nov 30, 2019

@wayilau I think it was caused by the wrong YAML format. Has been fixed. Now, you need to make your tests passed.

I think i know why. I missed some commits.

@dmsolr
Copy link
Member

dmsolr commented Nov 30, 2019

It looks like good.
Let's wait for this result. :)

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.

LGTM

Copy link
Member

@wu-sheng wu-sheng left a comment

Choose a reason for hiding this comment

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

LGTM. 53 MySQL Cases, crazy...

@wu-sheng
Copy link
Member

@wayilau I don't get any reply from you about the mail. Are you planning to do?

@wu-sheng wu-sheng merged commit c0c898c into apache:master Nov 30, 2019
@wayilau
Copy link
Member Author

wayilau commented Nov 30, 2019

@wayilau I don't get any reply from you about the mail. Are you planning to do?

@wu-sheng
I did not receive any emails from you yet. So i don't understand ..

@wu-sheng
Copy link
Member

Could you send a mail to me? I send you a mail based on your GitHub homepage mail, maybe it is incorrect? Mind is correct, you could send a mail to there.

@wayilau
Copy link
Member Author

wayilau commented Nov 30, 2019

Could you send a mail to me? I send you a mail based on your GitHub homepage mail, maybe it is incorrect? Mind is correct, you could send a mail to there.

I am sorry, i just saw mail from my rubbish mail box. May be the mail settings is limit by the commany. I will read the mail first. @wu-sheng

@wayilau wayilau deleted the dev3 branch November 30, 2019 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature test Test requirements about performance, feature or before release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants