-
Notifications
You must be signed in to change notification settings - Fork 52
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
Fix PhantomJS failure for s390x & add support for Github actions. #51
Conversation
Skipping TC execution for s390x Added s390x support in GitHub Actions Signed-off-by: Prabhav Thali <Prabhav.Thali1@ibm.com>
Hello @prabhav-thali , thanks for the PR! Have you considered not to install phantomjs and other dev dependecies for |
@akurinnoy Thanks for the suggestion. I tried using |
@akurinnoy Any update on the merge? |
apache.Dockerfile
Outdated
@@ -10,12 +10,16 @@ | |||
|
|||
FROM docker.io/node:8.16.2 as builder | |||
|
|||
RUN if [ "$(uname -m)" = "s390x" ]; then apt-get update; apt-get install -y phantomjs; fi |
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.
hello, it looks like too hardcoded for a specific arch
maybe you need to provide a DOCKER_ARG to have skip-tests option and ignore optional
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.
@benoitf If my understanding is correct, passing DOCKER_ARG can impact Github Actions and will need to be taken care of with additional docker buildx build
commands. Please let me know your thoughts.
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.
@benoitf are you OK to merge this PR as is? or do you want changes?
cc82946
to
7f680f1
Compare
Signed-off-by: Prabhav Thali <Prabhav.Thali1@ibm.com>
Signed-off-by: Prabhav Thali Prabhav.Thali1@ibm.com
What does this PR do?
Skips TC execution for s390x. Installation of phantomjs-prebuilt fails as phantomjs binary for s390x is not available at the provided CDN URLs. Also, we tried installing phantomjs using
apt-get install phantomjs
but phantomjs crashes while executingyarn test
. So currently skippingyarn test
for s390x.Also, added s390x support in GitHub Actions.
What issues does this PR fix or reference?
This PR is part of this initiative and also fixes phantomjs failure for s390x mentioned in #16983.