MIGENG-365 - End2end integration test #20

jonathanvila · 2019-07-24T17:44:06Z

https://issues.redhat.com/browse/MIGENG-365

This E2E test is based in these assumptions :

creating a real Kafka broker
creating a real ActiveMQ Artemis broker
creating a real Ingress instance
creating a real KIE instance

To do this it's used :

TestContainers : to create containers for Kafka, AMQ Artemis, Ingress, PostgreSQL, Minio, Localstack, KIE [https://www.testcontainers.org]

Added Sonar + Jacoco execution

…y test Camel

Added JUnit 5 and AssertJ to pom Aplied some clean-code suggestions

Added support for JUnit 5 on old maven versions Added constructors on model Fixed AnalyticsCalculatorTest test Added Sonar info to README

Added lombok file to add Generated annotation that will be avoided by JaCoCo Added sample file to test multipart Added test for the multipart Removed accessors/modifiers for cloudforms classes Cleaned CustomizedMultipartDataFormat

Renamed test methods

Moved back to JUnit 4 as Camel Test support doesnt support JUnit 5 yet

Added very simple test for direct:store

Added test for direct-upload with individual files and zip file

Added test for direct:insights route

minor change on xmlroutestest

# Conflicts: # lombok.config # pom.xml # src/test/java/org/jboss/xavier/integrations/route/XmlRoutesTest.java

added cloudforms export file

…me "issues"

…sed code

removed testing route

pom.xml

carlosthe19916 · 2019-12-19T09:54:55Z

@jonathanvila Thanks for the last commit, I can see that the last build in Travis failed with the same error I've found locally: Condition with lambda expression in org.jobss.xavier.integrations.EndToEndTest was not fulfilled within 10000 milliseconds

This is the Travis Log:
log.txt

Here is a screenshot from my local machine.

I've tested locally and it seems the error appears when my Laptop is slow (When I don't have all the CPU resources available). I guess it is because the containers are taking too much time for starting and it causes the problem.

I wonder if increasing the time from 10000 will solve the problem or not, what do you think?

carlosthe19916 · 2019-12-19T10:01:30Z

@jonathanvila I've tried to deploy your PR in CI and the pod is not able to start because Could not resolve placeholder 'minio.host' in value "${minio.host}" error, could you please make the necessary changes to application.properties, application-test.properties, or any other file you require to make it deployable in CI?

Here is the log of CI:
analytics-integration-276-hgvrd.log

fixed not available minio.host prop on runtime

carlosthe19916 · 2019-12-19T10:54:32Z

src/main/java/org/jboss/xavier/analytics/pojo/output/workload/summary/ScanRunModel.java

-        String typeString = "Virt Platform";
-        return typeString + (type?" + SmartState": "");
+    public Boolean getType() {
+        return type;


As we agreed on, this change will require to create a Ticket for changing the UI so there are no visual changes. For now, I can confirm that this block of code will change the Scans run table of the Workload Migration summary report... I'm writing just not to forget to create the ticket

@jonathanvila and @carlosthe19916 sorry, just for my benefit.
The old getType method's implementation has changed: it returns a Boolean and not the String the UI was showing (highlighted in the above screenshot).
Is there, in this PR, a new method that return the same String the old getType method provided?

No , there is no method to return the string needed. The UI ticket is only going to show the text that comes from the report or it will convert the boolean to the String it was being sent ? When we said the UI was going to change because of this change I assumed it was because the UI would be in charge of that change. If it's the backend who has to send the string in the report , then that method is missing at the moment.

Discussed a little bit more about this with @carlosthe19916 because to me having a boolean type field was not clear what it referred to.
The previous String type was providing the Type label value to the shown in the UI (screenshot above) but now a boolean value is hard to understand what it refers to just calling it type.
I think it could be improved having the field to be renamed to smartStateEnabled so that it's clear the boolean refers to Smart State begin enabled in the analysis.

Then we will change the UI to manage this boolean field accordingly (https://issues.redhat.com/browse/MIGENG-370)

@mrizzi @jonathanvila I've created a PR (on Jonathan's branch) for renaming type to smartStateEnabled; however, this change is just using @JsonProperty("smartStateEnabled") so it won't affect the current database model. It is not trivial to change the database model since hibernate won't update the database automatically. for us. Maybe in the future, we can introduce a database version control manager like flyway

Before it was also a boolean on the mutator setType , and it was not clear at all too, then we had a String accessor getType not returning the internal value but transforming it using a hard coded string. So we lived with unclearity also before.

My suggestion is to solve the type of the attribute now and in a separate ticket rename the attribute, and execute the rename into the database or implement Flyway (or any other DB change tool ) in order to move forward with this PR

…Configuration to any profile except test

src/main/java/org/jboss/xavier/integrations/route/MainRouteBuilder.java

…the test

src/main/java/org/jboss/xavier/integrations/SpringConfiguration.java

carlosthe19916

A couple of more comments

carlosthe19916 · 2019-12-19T14:48:55Z

src/test/java/org/jboss/xavier/integrations/route/MainRouteBuilder_S3Test.java

@@ -28,14 +28,14 @@
    @Test
    @Ignore
    public void s3Test() throws Exception {


I know this test is ignored even in the master branch but if this Test Class is used somehow I would suggest to fix it and remove the @ignore; otherwise, we can delete this in order to clean the source code. What do you think?

Well this test is here because it was handy to test the S3 component. We can remove it , but I wanted to leave it there in order to have the code if we need to test more things with the component.

carlosthe19916 · 2019-12-19T14:49:11Z

.travis.yml

+  - docker-compose --version
+  - docker info
+  - curl -fsSL https://download.docker.com/linux/ubuntu/gpg | sudo apt-key add -
+  - sudo add-apt-repository "deb [arch=amd64] https://download.docker.com/linux/ubuntu $(lsb_release -cs) stable"
+  - sudo apt-get update
+  - sudo apt-get -y -o Dpkg::Options::="--force-confnew" install docker-ce
+  - sudo rm /usr/local/bin/docker-compose
+  - curl -L https://github.com/docker/compose/releases/download/1.22.0/docker-compose-Linux-x86_64 > docker-compose
+  - chmod +x docker-compose
+  - sudo mv docker-compose /usr/local/bin
+  - docker-compose --version
+  - docker info


I would suggest using the default versions of Travis so when Travis will upgrade the Docker and Docker Compose versions we will have it without any change in our side

Well, I wanted to fix a version that we know it works.... yes using the default we will have updates, for the good and for the bad, as we dont know if Testcontainers will cope with the same version of the docker installed in Travis.

carlosthe19916 · 2019-12-19T14:52:42Z

...egrations/route/WorkloadSummaryReportRoutes_DirectCalculateVMWorkloadInventoryModelTest.java

@@ -372,11 +372,9 @@ public void DirectCalculateVMWorkloadInventoryModel_ShouldPersistScanRunModel()
        Assert.assertNotNull(scanRunModels);
        Assert.assertEquals(2, scanRunModels.size());

-        scanRunModels.stream().filter(model -> model.getId() % 2 == 0).forEach(srm ->
-                Assert.assertEquals("Virt Platform", srm.getType()));
+        Assert.assertTrue(scanRunModels.stream().filter(model -> model.getId() % 2 == 0).allMatch(srm -> !srm.getType()));


This is just a reminder to not forget to create the ticket for changing the UI.

carlosthe19916 · 2019-12-19T15:27:21Z

src/test/resources/drools-kie-compose.yml

@@ -0,0 +1,38 @@
+version: '2'


I might be wrong but it seems to me that this file is not used in the end2end tests. We can remove this file, What do you think?

This file is there because it helped to start the compose with KIE and test the commands. We can remove it , but I left it here in order to have the yaml if we need to do further tests with the compose.

carlosthe19916 · 2019-12-19T15:27:38Z

src/test/resources/end2end_test_docker_curl_commands.sh

@@ -0,0 +1,99 @@
+#!/usr/bin/env bash


I might be wrong but it seems to me that this file is not used in the end2end tests. We can remove this file, What do you think?

The same as with the kie compose. It was helpful to have it in order to recreate the infrastracture with the compose. We can remove it , but I thought it was interesting to have it.

src/test/java/org/jboss/xavier/integrations/EndToEndTest.java

carlosthe19916 · 2019-12-19T15:41:08Z

src/test/java/org/jboss/xavier/integrations/EndToEndTest.java

+    @Value("${minio.host:x}")
+    private String minio_host;
+
+    public static class Initializer implements ApplicationContextInitializer<ConfigurableApplicationContext> {


Since we have the limitation of writing just one Test method by Class, then I would suggest moving the Initializer class outside the EndToEndTest.java class since it might be used by future Test Class. What do you think?

As we discussed; my idea is to make some parts of the Class reusable by future End2End test classes. However, I agree that for now there are no other End2End tests in mind so we can postpone the refactoring of this class

As we talked, moving the initializer out of the class will convert it in a non static class .... and also it needs the values from the containers started ( ip and port ) there fore we need to have the initializer and the test class on sync. So I dont see much benefit on moving the class out and it will complicate things a bit.
Also, I think reusable is interesting but only when you need to reuse....

…ts to the test

…nmodel ScanRunModel: change `type` by `smartStateEnabled`

carlosthe19916

Thanks @jonathanvila for applying all the changes we discussed during the last couple of weeks :)... as far as I'm concern, this PR looks good to me so I'll approve it.

mrizzi

@jonathanvila i ran it locally 3 times and i found one issue with a command in a script.

mrizzi · 2020-01-08T15:37:51Z

src/test/resources/minio-bucket-creation-commands.sh

+/usr/bin/mc config host add myminio http://$1 BQA2GEXO711FVBVXDWKM uvgz3LCwWM3e400cDkQIH/y1Y4xgU4iV91CwFSPC;
+/usr/bin/mc mb myminio/insights-upload-perma;
+/usr/bin/mc mb myminio/insights-upload-rejected;
+/usr/bin/mc policy download myminio/insights-upload-perma;


it seem that locally this command didn't work for me since in the log it prints the usage output:

16:31:12.000 [tc-okhttp-stream-2142129482] INFO o.j.xavier.integrations.EndToEndTest - [MINIO-MC-LOG] STDOUT: Added `myminio` successfully. 16:31:12.000 [tc-okhttp-stream-2142129482] INFO o.j.xavier.integrations.EndToEndTest - [MINIO-MC-LOG] STDOUT: Bucket created successfully `myminio/insights-upload-perma`. 16:31:12.000 [tc-okhttp-stream-2142129482] INFO o.j.xavier.integrations.EndToEndTest - [MINIO-MC-LOG] STDOUT: Bucket created successfully `myminio/insights-upload-rejected`. 16:31:12.000 [tc-okhttp-stream-2142129482] INFO o.j.xavier.integrations.EndToEndTest - [MINIO-MC-LOG] STDOUT: Name: 16:31:12.000 [tc-okhttp-stream-2142129482] INFO o.j.xavier.integrations.EndToEndTest - [MINIO-MC-LOG] STDOUT: mc policy - manage anonymous access to buckets and objects 16:31:12.076 [tc-okhttp-stream-2142129482] INFO o.j.xavier.integrations.EndToEndTest - [MINIO-MC-LOG] STDOUT: USAGE: 16:31:12.077 [tc-okhttp-stream-2142129482] INFO o.j.xavier.integrations.EndToEndTest - [MINIO-MC-LOG] STDOUT: mc policy set [FLAGS] PERMISSION TARGET 16:31:12.077 [tc-okhttp-stream-2142129482] INFO o.j.xavier.integrations.EndToEndTest - [MINIO-MC-LOG] STDOUT: mc policy set-json [FLAGS] FILE TARGET 16:31:12.077 [tc-okhttp-stream-2142129482] INFO o.j.xavier.integrations.EndToEndTest - [MINIO-MC-LOG] STDOUT: mc policy get [FLAGS] TARGET 16:31:12.077 [tc-okhttp-stream-2142129482] INFO o.j.xavier.integrations.EndToEndTest - [MINIO-MC-LOG] STDOUT: mc policy get-json [FLAGS] TARGET 16:31:12.077 [tc-okhttp-stream-2142129482] INFO o.j.xavier.integrations.EndToEndTest - [MINIO-MC-LOG] STDOUT: mc policy list [FLAGS] TARGET

… they are wrong and apparently not needed

mrizzi

Latest changes worked for me.
Thanks @jonathanvila for this improvement to help us improve code's stability and maintainability.
Thanks @bsideup for the help and support 👍

jonathan added 30 commits May 30, 2019 18:20

WINDUP-2398 changed mimetype and created sample available json

9fcebfb

changed kafka topic

2f8f64e

changed kafka topic and category

981edc6

refactored code

b891656

replaced seda by direct as it seemed not to work

fc06118

Converted calculator into Bean

1c84a1a

Added Sonar + Jacoco execution

Created first test for a simple route in order to know how to properl…

91ed29f

…y test Camel

Created unit test for AnalysticsCalculator

2e95cb0

Added JUnit 5 and AssertJ to pom Aplied some clean-code suggestions

Removed sonar token from pom

39cd374

WINDUP-2398

77ae079

Added support for JUnit 5 on old maven versions Added constructors on model Fixed AnalyticsCalculatorTest test Added Sonar info to README

WINDUP-2398

61517e4

Added lombok file to add Generated annotation that will be avoided by JaCoCo Added sample file to test multipart Added test for the multipart Removed accessors/modifiers for cloudforms classes Cleaned CustomizedMultipartDataFormat

WINDUP-2398

5e9b123

Renamed test methods

WINDUP-2398

bf4e401

Renamed test methods

WINDUP-2398

4d8be22

Moved back to JUnit 4 as Camel Test support doesnt support JUnit 5 yet

WINDUP-2398

612d3b0

Added very simple test for direct:store

WINDUP-2398

1454c30

Added test for direct-upload with individual files and zip file

WINDUP-2398

7a300b2

Added test for direct:insights route

WINDUP-2398

1903e64

minor change on xmlroutestest

WINDUP-2398 Applied changes from travis PR

153b861

Merge branch 'master' into WINDUP-2398-upload-api

56dfc3e

# Conflicts: # lombok.config # pom.xml # src/test/java/org/jboss/xavier/integrations/route/XmlRoutesTest.java

WNDUP-2398 added test for kafka listener route

423ad0c

WNDUP-2398 added a test for the direct-download route ( not finished )

17cb16f

WNDUP-2398 fixed test for direct:download route

d5a2a89

WNDUP-2398 added test for direct:calculate

4f6b001

added cloudforms export file

WNDUP-2398 testing the rest endpoint

053ad94

WNDUP-2398 testing the rest-upload endpoint

ad783cd

WNDUP-2398 testing the rest report endpoint

0ea21b7

WNDUP-2398 switched to regular endpoints for rest, as rest DSL has so…

38dd83e

…me "issues"

WNDUP-2398 renamed kie helper methods with no Random, and removed unu…

bad57c2

…sed code

WNDUP-2404 testing call-kie-extract-reports route

278c92b

removed testing route

carlosthe19916 reviewed Dec 16, 2019

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

jonathan added 2 commits December 18, 2019 17:59

MIGENG-365 removed github pipeline

4bdc0b3

MIGENG-365 cleanup downloaded files

f82cfd2

MIGENG-365 added timouts to the test properties and increased the value

4344cf4

fixed not available minio.host prop on runtime