You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2020/03/16 04:29:34 UTC
[GitHub] [skywalking-nginx-lua] dmsolr opened a new pull request #24:
[WIP]Add e2e standone test
dmsolr opened a new pull request #24: [WIP]Add e2e standone test
URL: https://github.com/apache/skywalking-nginx-lua/pull/24
Nginx Lua Agent reports to Mock Collector and validates.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] dmsolr commented on a change in pull request
#24: Add e2e standone test
Posted by GitBox <gi...@apache.org>.
dmsolr commented on a change in pull request #24: Add e2e standone test
URL: https://github.com/apache/skywalking-nginx-lua/pull/24#discussion_r393536025
##########
File path: .github/workflows/e2e.yaml
##########
@@ -42,6 +42,6 @@ jobs:
maven-version: 3.6.1
- name: Set environment
run: export MAVEN_OPTS='-Dmaven.repo.local=~/.m2/repository -XX:+TieredCompilation -XX:TieredStopAtLevel=1 -XX:+CMSClassUnloadingEnabled -XX:+UseConcMarkSweepGC -XX:-UseGCOverheadLimit -Xmx3g'
- - name: Compile & Install Test Codes
+ - name: E2E Test
run: |
- mvn -f ./test/e2e/validator/pom.xml clean verify
+ mvn -f ./test/e2e/pom.xml package verify
Review comment:
I have tried, but not IT work. Maybe I miss something.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng commented on issue #24: [WIP]Add
e2e standone test
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #24: [WIP]Add e2e standone test
URL: https://github.com/apache/skywalking-nginx-lua/pull/24#issuecomment-599391139
Is this in WIP or ready to review?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] kezhenxu94 commented on a change in pull
request #24: Add e2e test with mock collector
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #24: Add e2e test with mock collector
URL: https://github.com/apache/skywalking-nginx-lua/pull/24#discussion_r393599656
##########
File path: test/e2e/agent-test-tools/docker/Dockerfile
##########
@@ -0,0 +1,14 @@
+FROM openjdk:8
+
+WORKDIR /usr/local/skywalking-nginx-lua
+
+COPY run.sh /
+RUN chmod +x /run.sh
+
+COPY skywalking-validator.jar ./agent-test-tools/skywalking-validator-tool.jar
+COPY skywalking-mock-collector.tar.gz ./agent-test-tools/
+
+RUN tar -xvf ./agent-test-tools/skywalking-mock-collector.tar.gz -C agent-test-tools/
+RUN chmod +x /usr/local/skywalking-nginx-lua/agent-test-tools/skywalking-mock-collector/bin/collector-startup.sh
+
+CMD ["/usr/local/skywalking-nginx-lua/agent-test-tools/skywalking-mock-collector/bin/collector-startup.sh"]
Review comment:
Since you have a `WORKDIR` at line 3, the file paths can be shorten, they are too long
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] kezhenxu94 commented on a change in pull
request #24: Add e2e test with mock collector
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #24: Add e2e test with mock collector
URL: https://github.com/apache/skywalking-nginx-lua/pull/24#discussion_r393602335
##########
File path: test/e2e/e2e-with-mock-collector/docker/conf.d/nginx.conf
##########
@@ -0,0 +1,87 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+# This nginx.conf is designed and written for local dev environments
+# It will use the blocking startup mode and console logging
+worker_processes 1;
+daemon off;
+error_log /dev/stdout debug;
+
+events {
+ worker_connections 1024;
+}
+http {
+ lua_package_path "/usr/local/skywalking-nginx-lua/lib/skywalking/?.lua;;";
+ # Buffer represents the register inform and the queue of the finished segment
+ lua_shared_dict tracing_buffer 100m;
+
+ # Init is the timer setter and keeper
+ # Setup an infinite loop timer to do register and trace report.
+ init_worker_by_lua_block {
+ local metadata_buffer = ngx.shared.tracing_buffer
+
+ metadata_buffer:set('serviceName', 'e2e-test-with-mock-collector')
+ -- Instance means the number of Nginx deloyment, does not mean the worker instances
Review comment:
```suggestion
-- Instance means the number of Nginx deployment, does not mean the worker instances
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng commented on issue #24: [WIP]Add
e2e standone test
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on issue #24: [WIP]Add e2e standone test
URL: https://github.com/apache/skywalking-nginx-lua/pull/24#issuecomment-599825207
I think he wanted to put `standalone`, meaning there is only one nginx running?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] wu-sheng merged pull request #24: Add e2e
test with mock collector
Posted by GitBox <gi...@apache.org>.
wu-sheng merged pull request #24: Add e2e test with mock collector
URL: https://github.com/apache/skywalking-nginx-lua/pull/24
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] kezhenxu94 commented on a change in pull
request #24: [WIP]Add e2e standone test
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #24: [WIP]Add e2e standone test
URL: https://github.com/apache/skywalking-nginx-lua/pull/24#discussion_r393386637
##########
File path: .github/workflows/e2e.yaml
##########
@@ -42,6 +42,6 @@ jobs:
maven-version: 3.6.1
- name: Set environment
run: export MAVEN_OPTS='-Dmaven.repo.local=~/.m2/repository -XX:+TieredCompilation -XX:TieredStopAtLevel=1 -XX:+CMSClassUnloadingEnabled -XX:+UseConcMarkSweepGC -XX:-UseGCOverheadLimit -Xmx3g'
- - name: Compile & Install Test Codes
+ - name: E2E Test
run: |
- mvn -f ./test/e2e/validator/pom.xml clean verify
+ mvn -f ./test/e2e/pom.xml package verify
Review comment:
goal `package` already includes `verify` IIRC, so you may verify twice
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] kezhenxu94 commented on a change in pull
request #24: Add e2e test with mock collector
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #24: Add e2e test with mock collector
URL: https://github.com/apache/skywalking-nginx-lua/pull/24#discussion_r393598804
##########
File path: test/e2e/agent-test-tools/docker/Dockerfile
##########
@@ -0,0 +1,14 @@
+FROM openjdk:8
+
+WORKDIR /usr/local/skywalking-nginx-lua
+
+COPY run.sh /
+RUN chmod +x /run.sh
+
+COPY skywalking-validator.jar ./agent-test-tools/skywalking-validator-tool.jar
+COPY skywalking-mock-collector.tar.gz ./agent-test-tools/
+
+RUN tar -xvf ./agent-test-tools/skywalking-mock-collector.tar.gz -C agent-test-tools/
Review comment:
`COPY` + `tar -xvf` can be replaced with `ADD`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] kezhenxu94 commented on a change in pull
request #24: Add e2e test with mock collector
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #24: Add e2e test with mock collector
URL: https://github.com/apache/skywalking-nginx-lua/pull/24#discussion_r393603204
##########
File path: test/e2e/e2e-with-mock-collector/src/test/java/org/apache/skywalking/e2e/DataAssertITCase.java
##########
@@ -0,0 +1,79 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package org.apache.skywalking.e2e;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.util.concurrent.TimeUnit;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.entity.InputStreamEntity;
+import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+
+public class DataAssertITCase {
+ private CloseableHttpClient client = HttpClientBuilder.create().build();
+ private static final int MAX_RETRY_TIMES = 5;
+ private String validationEntry;
+ private String serviceEntry;
+
+ @Before
+ public void setup() throws IOException {
+ serviceEntry = System.getProperty("service.entry");
+ validationEntry = System.getProperty("validation.entry");
+ }
+
+ @Test
+ public void verify() throws IOException, InterruptedException {
+ TimeUnit.SECONDS.sleep(5L); // Wait Nginx Lua Agent available.
Review comment:
potential instability
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] dmsolr commented on issue #24: Add e2e
standone test
Posted by GitBox <gi...@apache.org>.
dmsolr commented on issue #24: Add e2e standone test
URL: https://github.com/apache/skywalking-nginx-lua/pull/24#issuecomment-599960644
Hi, It is ready to review, please.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] dmsolr commented on issue #24: [WIP]Add e2e
standone test
Posted by GitBox <gi...@apache.org>.
dmsolr commented on issue #24: [WIP]Add e2e standone test
URL: https://github.com/apache/skywalking-nginx-lua/pull/24#issuecomment-599906020
Yes, it is `Standalone`. Running Nginx only without Mock Collector/OAP.
It is not an e2e test indeed. Maybe it is called an integration test.
I am confused about whether we need it when we had an e2e test.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] kezhenxu94 commented on a change in pull
request #24: Add e2e test with mock collector
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #24: Add e2e test with mock collector
URL: https://github.com/apache/skywalking-nginx-lua/pull/24#discussion_r393597892
##########
File path: test/e2e/agent-test-tools/docker/Dockerfile
##########
@@ -0,0 +1,14 @@
+FROM openjdk:8
Review comment:
license header
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [skywalking-nginx-lua] dmsolr commented on issue #24: [WIP]Add e2e
standone test
Posted by GitBox <gi...@apache.org>.
dmsolr commented on issue #24: [WIP]Add e2e standone test
URL: https://github.com/apache/skywalking-nginx-lua/pull/24#issuecomment-599906426
It is still WIP. @wu-sheng
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services