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

add equivalent dependencies for rhel/centos #547

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions source/guides/guides/continuous-integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,16 @@ See our {% url 'examples' docker %} for additional information on our maintained

If you are not using one of the above CI providers then make sure your system has these dependencies installed.

## Ubuntu / Debian
```shell
apt-get install xvfb libgtk2.0-0 libnotify-dev libgconf-2-4 libnss3 libxss1 libasound2
Copy link
Contributor

Choose a reason for hiding this comment

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

we should add -y here too

```

## RHEL / CentOS
```shell
yum install xorg-x11-server-Xvfb gtk2 libnotify GConf2 nss libXScrnSaver alsa-lib
Copy link
Contributor

Choose a reason for hiding this comment

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

we should add libXtst and a -y so you don't have to manually approve them

Copy link
Author

@joshsleeper joshsleeper Apr 25, 2018

Choose a reason for hiding this comment

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

I was just matching package for package with the existing apt-get call, which doesn't seem to install libXtst.
Curious, is libXtst required to run the Electron version as well, or only Chrome Stable?

Regardless, if you want me to add libXtst, I'd like to add the equivalent dependency to the apt-get command as well.
Any idea what those packages are, or do I need to hunt em down?

Also, I'm fine adding the -y if you really want it, but I'm of the opinion that this seems to be code that a user would copy and paste to run. As such, I kinda feel like it's more appropriate to let the user manually accept the resulting dependencies. Thoughts?

Copy link
Author

Choose a reason for hiding this comment

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

@bkucera any thoughts on my comments?

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe Ubuntu ships with that dependency already installed.
Also since must users come from the npm world, where packages are arguably less vetting and verified, and does not ask to confirm every package individually, I think -y is an okay addition

Copy link
Contributor

Choose a reason for hiding this comment

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

And yes, it's required for electron

Copy link
Author

Choose a reason for hiding this comment

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

@jennifer-shehane
any input on this?

  1. should libXtst be added to both lines?
  2. do you agree or disagree with adding -y to the commands?

I'm unsure if the libXtst dep is an actual requirement or not given that all of my systems have it pre-installed and I can't remove it without breaking other dep chains.

I'm also hesitant to add -y to the commands unless the core team likes that approach. @bkucera's stance about most of the users being npm friendly makes sense, but I would counter-point that just because they're npm friendly doesn't mean that the shell should suddenly act like npm. ;)
Either way, I'd defer to your stance on that one.

Copy link
Contributor

Choose a reason for hiding this comment

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

So I was looking at the docs, and I think instead of adding every dependency here in Markdown, without really testing it, we could just

  • keep existing list of Debian dependencies (since this is the most common image I believe)
  • point at our https://github.com/cypress-io/cypress-docker-images repo where the dependencies are actually used to make and test images. In these docs we can say "for the up-to-date list see Docker image "

If a library is needed to run Cypress, one can always open an issue in cypress-docker-images with a test case that fails to execute, and adding the library solves

Copy link
Author

@joshsleeper joshsleeper May 1, 2018

Choose a reason for hiding this comment

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

what about something like this?

# Dependencies

If you are not using one of the above CI providers then make sure your system has the required dependencies installed.

An up-to-date list of required dependencies for supported platforms can be found in the `Dockerfile` for each platform in the [`cypress-io/cypress-docker-images`](https://github.com/cypress-io/cypress-docker-images) repository

I'm not sure I see the value in keeping only the debian list if we're going to suggest they check the Dockerfiles for an up-to-date list anyway, but if you feel strongly about it I can do that.

edit

upon further thought, I definitely don't like keeping the debian dep list. to your own point, keeping a list of exact deps, untested, here in Markdown, doesn't make a lot of sense. rather than have our debian dep list definitely become out of date at some point, i think we're much better off just directing them to the cypress-docker-images repo so we never have out of date dep docs again.

yay? nay?

```

# Recording Tests in CI

Cypress can record your tests running and make them available in our {% url 'Dashboard' https://on.cypress.io/dashboard %}.
Expand Down