Navigation Menu

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

[OSSM-1216] cypress container #6029

Merged
merged 6 commits into from May 5, 2023

Conversation

ScriptingShrimp
Copy link
Contributor

First iteration of docker file for cypress execution in container

@ScriptingShrimp ScriptingShrimp self-assigned this Apr 13, 2023
@ScriptingShrimp ScriptingShrimp added the test: front-end/cypress PR adds/updates front-end tests (unit and/or cypress automation ) label Apr 13, 2023
Copy link
Contributor

@FilipB FilipB left a comment

Choose a reason for hiding this comment

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

Not sure about this but it might make better sense to put the Dockerfile into cypress folder? Instead of 'deploy/docker' folder?

deploy/docker/Dockerfile-cypress Outdated Show resolved Hide resolved
deploy/docker/Dockerfile-cypress Show resolved Hide resolved
deploy/docker/Dockerfile-cypress Outdated Show resolved Hide resolved
deploy/docker/Dockerfile-cypress Outdated Show resolved Hide resolved
@ScriptingShrimp
Copy link
Contributor Author

Not sure about this but it might make better sense to put the Dockerfile into cypress folder? Instead of 'deploy/docker' folder?

I asked team where to put it, looks like everyone is fine with deploy/dockerfile folder. I also saw make targets navigate here for other docker related things.

@ScriptingShrimp ScriptingShrimp added the do not merge A PR is not ready to merge label May 3, 2023
@ScriptingShrimp ScriptingShrimp marked this pull request as ready for review May 3, 2023 21:20
Comment on lines +1 to +4
_output
operator
deploy
frontend/node_modules
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this will affect other Dockerfiles in this folder, is that safe @FilipB

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be safe as other images are build using CONTEXT ${OUTDIR}/docker, e.g. https://github.com/kiali/kiali/blob/master/make/Makefile.container.mk#L17

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand it precisely, but I believe you 😄

@FilipB FilipB self-requested a review May 5, 2023 10:36
Copy link
Contributor

@FilipB FilipB left a comment

Choose a reason for hiding this comment

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

LGTM

@FilipB FilipB merged commit 595cbb1 into kiali:master May 5, 2023
5 checks passed
ScriptingShrimp added a commit to ScriptingShrimp/kiali that referenced this pull request May 5, 2023
* initial commit for cypress container

* removin old fix for chrome

* cleanup and yarn --force

* removing user

* OSSM-3670: updating cypress dockerfile to make it work on OCP

* copy also hack scripts which will be used to install demo apps

---------

Co-authored-by: Filip Brychta <fbrychta@redhat.com>
FilipB added a commit that referenced this pull request May 5, 2023
* initial commit for cypress container

* removin old fix for chrome

* cleanup and yarn --force

* removing user

* OSSM-3670: updating cypress dockerfile to make it work on OCP

* copy also hack scripts which will be used to install demo apps

---------

Co-authored-by: Filip Brychta <fbrychta@redhat.com>
pantianying pushed a commit to pantianying/kiali that referenced this pull request Jun 14, 2023
* initial commit for cypress container

* removin old fix for chrome

* cleanup and yarn --force

* removing user

* OSSM-3670: updating cypress dockerfile to make it work on OCP

* copy also hack scripts which will be used to install demo apps

---------

Co-authored-by: Filip Brychta <fbrychta@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge A PR is not ready to merge test: front-end/cypress PR adds/updates front-end tests (unit and/or cypress automation )
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants