-
Notifications
You must be signed in to change notification settings - Fork 122
Plugin test based on skywalking-infra-e2e #122
Conversation
Codecov Report
@@ Coverage Diff @@
## master #122 +/- ##
==========================================
+ Coverage 67.48% 69.33% +1.85%
==========================================
Files 17 17
Lines 735 887 +152
==========================================
+ Hits 496 615 +119
- Misses 195 224 +29
- Partials 44 48 +4
Continue to review full report at Codecov.
|
@mrproliu @wu-sheng @kezhenxu94 Please review :) CC @kagaya85 we tested the plugin via skywalking-infra-e2e |
plugins/http/test/docker-compose.yml
Outdated
|
||
services: | ||
mockoap: | ||
image: arugaldocker/skywalking-mock-collector:1.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should built the image on the fly (I prefer) or release this docker officially in Apache, otherwise other repo will copy and paste to use this image, which is bad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, this is what I used for testing. I forgot to change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @dmsolr do you have time to finish this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need an official release for this. otherwise, arugaldocker/skywalking-mock-collector is a potential trademark issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arugal Do you want to initiate a release for this? Mostly, we need to resolve the license and notice files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@arugal Do you want to initiate a release for this? Mostly, we need to resolve the license and notice files.
OK, I will finish this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
via ghcr.io/apache/skywalking-agent-test-tool/mock-collector:b01d4137fed862338d78a73b23435ccf425f9962
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work. Thanks for adopting skywalking-infra-e2e so quickly!!
plugins/http/test/expected.data.yml
Outdated
|
||
segmentItems: | ||
{{- range .segmentItems }} | ||
{{- if or (eq .serviceName "http-client") (eq .serviceName "http-server")}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a feeling, there is abuse about if or
. Such as we use multiple services' segments together, and mixing component names, operation names, with or without reference.
We should make sure the expected data is as strict as possible. Check how we check in the plugin test tool, all things should be separated and checked one by one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Glad to see infra-e2e used in the project:)
@arugal @kezhenxu94 @mrproliu Once we have several more e2e tests by using this tool, it is time to discuss its version number and run a release. Then we should write 1-2 blogs about how it works, and how other projects could adopt it. |
No description provided.