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

Rabbitmq plugin test migration #3788

Merged
merged 11 commits into from Nov 15, 2019

Conversation

viswaramamoorthy
Copy link
Contributor

Please answer these questions before submitting pull request

  • Why submit this pull request?

  • Bug fix

  • New feature provided

  • Improve performance

  • Related issues


Bug fix

  • Bug description.
    Migrated RabbitMQ plugin e2e test cases

  • How to fix?


New feature or improvement

  • Describe the details and related test reports.

@wu-sheng
Copy link
Member

wu-sheng commented Nov 5, 2019

Add your job to Jenkinsfile-Agent-Test file, workload 1 group. Otherwise, this would not be tested in CI.

@wu-sheng
Copy link
Member

wu-sheng commented Nov 5, 2019

FYI @arugal @Aderm

@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 5, 2019
@viswaramamoorthy viswaramamoorthy force-pushed the 3753-rabbitmq-e2e-tests branch 2 times, most recently from 4d8964c to 7167d88 Compare November 5, 2019 15:41
steps {
sh 'bash test/plugin/run.sh rabbitmq-scenario'
}
}
Copy link
Member

@arugal arugal Nov 5, 2019

Choose a reason for hiding this comment

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

Missing Test Case Report, please refer to the local-test-and-pull-request-to-the-upstream .

Copy link
Member

Choose a reason for hiding this comment

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

@arugal I helped on fixing this part.

@kezhenxu94 kezhenxu94 changed the title Rabbitmq plugin e2e test migration Rabbitmq plugin test migration Nov 6, 2019
<id>spring-milestones</id>
<url>http://repo.spring.io/milestone</url>
</pluginRepository>
</pluginRepositories>
Copy link

Choose a reason for hiding this comment

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

does this pluginRepositories needs?

Copy link
Member

Choose a reason for hiding this comment

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

Please notice this, I don't think we need spring repo. Maven central should be enough, right?

Copy link
Member

Choose a reason for hiding this comment

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

missing

Copy link
Member

Choose a reason for hiding this comment

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

@viswaramamoorthy could you remove this?

@viswaramamoorthy
Copy link
Contributor Author

I am not familiar with this framework. one of you can take over this PR.

@wu-sheng
Copy link
Member

wu-sheng commented Nov 7, 2019

@viswaramamoorthy The test is not hard, it is just demo codes with the real agent, and check the segments sent by the agent. You should understand it, it will help you to make sure the plugin is right.

@dmsolr
Copy link
Member

dmsolr commented Nov 7, 2019

Hi @viswaramamoorthy
It is almost to be good. follows the reviewer's suggestion and fix them.

@viswaramamoorthy
Copy link
Contributor Author

I ran rabbitmq-scenario locally using run.sh. I saw few basic errors.

is run.sh expected work locally for e.g. my laptop? I have docker installed.

@arugal
Copy link
Member

arugal commented Nov 7, 2019

I ran rabbitmq-scenario locally using run.sh. I saw few basic errors.

is run.sh expected work locally for e.g. my laptop? I have docker installed.

Hello, has healthcheck-url been added? this is the log for workload1-group1 because the failed due to a lack of healthcheck-url.

[2019-11-07T01:30:10.520Z] [ERROR] rabbitmq-scenario-5.5.1 url=http://localhost:8080/rabbitmq-case/case/healthcheck, status=HTTP/1.1 404 
 health check failed!

@viswaramamoorthy
Copy link
Contributor Author

Local error is not due to health check, it is complaining about missing or docker image permission. see below.

Pulling rabbitmq-scenario-5.5.1-local (skywalking/agent-test-jvm:local)...
ERROR: The image for the service you're trying to recreate has been removed. If you continue, volume data could be lost. Consider backing up your data before continuing.

Anyway, I have added healthcheck and made changes to log messages. check

@arugal
Copy link
Member

arugal commented Nov 7, 2019

Local error is not due to health check, it is complaining about missing or docker image permission. see below.

Run locally use bash test/plugin/run.sh -f rabbitmq-scenario

Anyway, I have added healthcheck and made changes to log messages. check

Good, let's wait for the test result.

Todo list:

  1. merge the master branch code
  2. modify Jenkinsfile-Agent-Test#Test Cases Report
  3. update The elapsed time list of plugins according to test results
  4. ...

@viswaramamoorthy
Copy link
Contributor Author

@arugal That is the issue. run,sh is an issue. build_id has to be defaulted to latest so the built docker image can be run. It is defaulted to "local" because of this, docker compose generated looks for " skywalking/agent-test-jvm:local" that never exists and hence container not starting

I have shown the problem in bold below. check

build_id="local"
......
print_help() {
echo "Usage: run.sh [OPTION] SCENARIO_NAME"
echo -e "\t-f, --force_build \t\t do force to build Plugin-Test tools and images"
echo -e "\t--build_id, \t\t\t specify Plugin_Test's image tag. Defalt: latest"

@arugal
Copy link
Member

arugal commented Nov 7, 2019

ha, use bash test/plugin/run.sh -f rabbitmq-scenario set the BUILD_ID, maybe you need to pull the latest code?

[[ "$force_build" == "on" ]] && ${mvnw} -f ${home}/pom.xml clean package -DskipTests -DBUILD_NO=${BUILD_NO:=local} docker:build

@dmsolr
Copy link
Member

dmsolr commented Nov 7, 2019

Hi, @viswaramamoorthy I updated the default value of BUILD_ID without usage. (I have a mistake. In fact, it has deleted.)
I have merged the master into your pr. Now, you need to pull the latest code. and retry it.

@wu-sheng
Copy link
Member

wu-sheng commented Nov 8, 2019

Related to #3583

@wu-sheng
Copy link
Member

wu-sheng commented Nov 8, 2019

Please follow this, https://github.com/apache/skywalking/blob/master/docs/en/guides/Plugin-test.md#the-elapsed-time-list-of-plugins. And add your task into the fastest one when it is ready to be merged.

@arugal
Copy link
Member

arugal commented Nov 8, 2019

/run agent-plugin-test-1

@viswaramamoorthy
Copy link
Contributor Author

@arugal @wu-sheng I have pushed commit with code review comment changes.

@wu-sheng wu-sheng requested a review from dmsolr November 10, 2019 02:26
<id>spring-milestones</id>
<url>http://repo.spring.io/milestone</url>
</pluginRepository>
</pluginRepositories>
Copy link
Member

Choose a reason for hiding this comment

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

missing

@wu-sheng
Copy link
Member

@viswaramamoorthy Please follow the review.

@wu-sheng
Copy link
Member

@viswaramamoorthy Code frozen is over. Please keep the update, after all checked, this could be merged.

@viswaramamoorthy
Copy link
Contributor Author

viswaramamoorthy commented Nov 12, 2019 via email

@wu-sheng
Copy link
Member

 >I am away from my computer for couple of days. If someone can take that
final comment and fix, they can. Or I will send update in two days.

Got it, we will wait.

@wu-sheng wu-sheng added this to the 6.6.0 milestone Nov 13, 2019
@wu-sheng
Copy link
Member

@viswaramamoorthy Please solve the conflicts and update to the master branch.

@wu-sheng
Copy link
Member

@viswaramamoorthy you need to solve the conflict.

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.

@Aderm @dmsolr Please recheck. I helped on fixing the missing two issues. It should be good to go.

@arugal
Copy link
Member

arugal commented Nov 15, 2019

LGTM

@wu-sheng
Copy link
Member

/run agent-plugin-test-2

@wu-sheng wu-sheng merged commit d0a1cdd into apache:master Nov 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants