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.

Sign up for GitHub

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

Jump to bottom

FAI 658: Add e2e test suite into PR checks #1092

Merged
merged 25 commits into from
Feb 7, 2022
Merged

FAI 658: Add e2e test suite into PR checks #1092

merged 25 commits into from
Feb 7, 2022

Conversation

sjamboro
Copy link

@sjamboro sjamboro commented Nov 3, 2021 •
edited by Ginxo
Loading

@kie-ci
Copy link
Contributor

kie-ci commented Nov 3, 2021

The (build) kogito-apps check has failed. Please check the logs.

@sjamboro
Copy link
Author

sjamboro commented Nov 3, 2021

Jenkins retest this

@kie-ci
Copy link
Contributor

kie-ci commented Nov 3, 2021

The (build) kogito-apps check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Nov 3, 2021

The (build) kogito-apps check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Nov 8, 2021

The (build) kogito-apps check has failed. Please check the logs.

manstis
manstis reviewed Nov 8, 2021
View reviewed changes
Copy link
Contributor

@manstis manstis left a comment •
edited
Loading

Choose a reason for hiding this comment

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

It ran locally fine for me. Great stuff.

There remains the question of getting it to run under GitHub Actions.

It might also be worth investigating if we can get yarn run build:services to only compile kogito-apps/explainability and kogito-apps/trusty rather than the whole of kogito-apps.. just an optimisation that would speed-up the e2e quite a lot.

It might also be worth adding kogito-apps/ui-packages/packages/trusty/it-tests/docker-compose/.env to a .gitignore file since it is generated. WDYT?

ui-packages/pom.xml Outdated Show resolved Hide resolved
ui-packages/pom.xml Outdated Show resolved Hide resolved
.github/workflows/kogito-apps-pr.yml Outdated Show resolved Hide resolved
jstastny-cz
jstastny-cz requested changes Nov 8, 2021
View reviewed changes
Copy link
Contributor

@jstastny-cz jstastny-cz left a comment

Choose a reason for hiding this comment

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

I think the goal here should be to have both jenkins and github actions pipelines executing similarly. Can you also add the new step into jenkins pipeline configuration?

  • A few comments inline.

ui-packages/pom.xml Outdated Show resolved Hide resolved
ui-packages/pom.xml Outdated Show resolved Hide resolved
jiripetrlik
jiripetrlik approved these changes Nov 9, 2021
View reviewed changes
Copy link
Contributor

@jiripetrlik jiripetrlik left a comment

Choose a reason for hiding this comment

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

I have only two very minor comments.

ui-packages/packages/trusty/it-tests/docker-compose/build.sh Outdated Show resolved Hide resolved
ui-packages/packages/trusty/it-tests/e2e/TrafficViolation.ts Show resolved Hide resolved
@sjamboro sjamboro force-pushed the FAI-658 branch 2 times, most recently from 5aeec3e to bd583ca Compare November 10, 2021 19:57
@kie-ci
Copy link
Contributor

kie-ci commented Nov 10, 2021

The (build) kogito-apps check has failed. Please check the logs.

@sjamboro sjamboro force-pushed the FAI-658 branch 2 times, most recently from 672fd2b to 8b647e3 Compare November 12, 2021 12:07
@sjamboro
Copy link
Author

Jenkins retest this

@kie-ci
Copy link
Contributor

kie-ci commented Nov 12, 2021

The (build) kogito-apps check has failed. Please check the logs.

@r00ta
Copy link
Contributor

r00ta commented Nov 15, 2021 •
edited
Loading

Hi @manstis @sjamboro @jiripetrlik , what do you think if we add a new module where we run those tests?
In that way we do not have to change/maintain the gh actions/jenkins pipeline. Those tests expect some docker images to be built by the other modules, we could add them in the dependencies just to be sure they are already there. Something like

  1. create a new module
  2. add the dependencies
  3. run the docker-compose
  4. run the cypress tests via yarn like https://github.com/kiegroup/kogito-apps/blob/2b2f44745bcb14cfd72440fb02767f78a0c923c8/ui-packages/pom.xml#L115

wdyt?

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@kie-ci
Copy link
Contributor

kie-ci commented Nov 22, 2021

The (build) kogito-apps check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Nov 29, 2021

The (build) kogito-apps check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Dec 6, 2021

The (build) kogito-apps check has failed. Please check the logs.

Ginxo
Ginxo approved these changes Feb 2, 2022
View reviewed changes
Copy link
Contributor

@Ginxo Ginxo left a comment

Choose a reason for hiding this comment

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

LGTM

Sona Mala added 25 commits February 7, 2022 09:09
FAI-658: Add e2e test suite into PR checks
9441a6d
FAI-658: fix Traffic Violation test (CF)
302c60c
FAI-658: try to find the ui-packages folder
8fbb1d9
FAI-658: run always E2E test suite
a8947fc
FAI-658: debugg github pr checks
e42c595
FAI-658: new E2E maven module - PoC
3a3a05f
FAI-658: remove npm registry url
b9883e0
FAI-658: enable TrafficViolation test
22556c8
FAI-658: Update infinispan 11.0.4 to 13.0.2
23cb3e6
FAI-658: do not use docker-compose and define images in pom.xml
236b298
FAI-658: resolve problem in ENV addresses
8d65742 
It seems that docker-maven-plugin does partially support a network and we need to use just a container name instead of aliases.
FAI-658: log containers for kafka, zookeper and infinispan
6a82856
FAI-658: move th e infinispan config file into the resources folder
2f55645
FAI-658: remove the security tag from infinispan.xml
c031695
FAI-658: set docker registry
87f4fd7
FAI-658: log all containers
53f7d38
FAI-658: Use latest tag for zookeeper (deprecated schema in v3.4.6)
2888721
FAI-658: log services into files and set empty NPM reg
5267c6c
FAI-658: fix test:e2e:trusty execution in CI
0d29165 
It seems that CI adds the --registry option and cypress command does not recognize it. Echo just write the registry option on stdout.
FAI-658: archive artifacts
b07160b
FAI-658: Set the log level to DEBUG in the infinispan container
a74367b
FAI-658: wait to infinitispan server is listening on port
61ab6ec
FAI-658: clear unnecessary settings
859a581 
log4j2.xml was removed. The reason is that the config file  was upload into container but the log file does not contain more messages
aliases of containers is not supported in network by DMP. URL with alias does not work
repository of Docker was removed because it is set by CI workflow.
FAI-658: fix comments
e87be2a
FAI-658: notes about dependency on ui-packages in README and pom comment
f6a7afb
@sonarqubecloud
Copy link

sonarqubecloud bot commented Feb 7, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@danielezonca danielezonca merged commit 8bb921e into apache:main Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ginxo Ginxo Ginxo approved these changes

@jstastny-cz jstastny-cz jstastny-cz approved these changes

@jiripetrlik jiripetrlik jiripetrlik approved these changes

@r00ta r00ta r00ta approved these changes

@manstis manstis manstis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@sjamboro @kie-ci @r00ta @radtriste @kie-ci1 @Ginxo @jiripetrlik @manstis @jstastny-cz @danielezonca