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

Change localhost port for cypress tests and document how its used #257

Merged
merged 2 commits into from
Sep 30, 2018
Merged
Show file tree
Hide file tree
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
29 changes: 21 additions & 8 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,24 @@

You will need:

- Node.js and npm
- webpack
- [Yarn](https://yarnpkg.com/)
- [Webpack](https://webpack.js.org/)

Windows users will need additional setup to enable build capabilities in NPM.
From an administrative command window:

npm install --global --production windows-build-tools
```sh
yarn global add windows-build-tools
```

## Getting started

1. Fork the project
2. Clone your forked project by running `git clone git@github.com:{
YOUR_USERNAME }/shepherd.git`
3. Run `npm install` to install node modules
3. Run `yarn` to install node modules
4. Test that you can build the source by moving/renaming the existing `dist`
directory and running `npm run build`
directory and running `yarn build`
5. Assuming everything went well, you should now have a `dist` directory that
matches the one you moved in step 4

Expand All @@ -28,16 +30,27 @@ you can focus on writing relevant code. If there is a fix or feature you would l
to contribute, we ask that you take the following steps:

1. Most of the _editable_ code lives in the `src` directory while built code
will end up in the `dist` directory upon running `npm run build`.
will end up in the `dist` directory upon running `yarn build`.

2. Provide a thoughtful commit message and push your changes to your fork using
2. The demo app is served out of the `docs/welcome` directory. Running `yarn start` will open it in your browser and initiate a live-reloading session as you make changes.


## Opening Pull Requests

1. Please Provide a thoughtful commit message and push your changes to your fork using
`git push origin master` (assuming your forked project is using `origin` for
the remote name and you are on the `master` branch).

3. Open a Pull Request on GitHub with a description of your changes.
2. Open a Pull Request on GitHub with a description of your changes.


## Testing

All PRs, that change code functionality, are required to have accompanying tests.

### Acceptance Tests

Acceptance tests are run using [`cypress`](https://github.com/cypress-io/cypress). A number of different testing configurations can be found in [`package.json`](/package.json), but you can simply run `yarn test:ci:watch` to build your latest changes and begin running the tests inside a Chrome browser instance.

⚠️ The acceptance tests are set up to run on `localhost` port `9002`. If you'd like to change this port, make sure to change the `baseUrl` option inside of [`cypress.json`](/cypress.json), and change any references to port `9002` in [`package.json`](/package.json) accordingly.

3 changes: 2 additions & 1 deletion cypress.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,6 @@
"integrationFolder": "test/cypress/integration",
"pluginsFile": "test/cypress/plugins/index.js",
"supportFile": "test/cypress/support/index.js",
"video": false
"video": false,
"baseUrl": "http://localhost:9002"
}
12 changes: 0 additions & 12 deletions docs/welcome/css/welcome.css

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@
"posttest:ci": "nyc report --reporter=lcov --reporter=text",
"rewrite-paths": "replace 'SF:.*src' 'SF:src' coverage/lcov.info",
"start": "yarn watch",
"start-test-server": "http-server",
"start-test-server": "http-server -p 9002",
"test": "yarn test:ci",
"test:build": "cross-env NODE_ENV=test yarn build",
"test:ci": "yarn test:unit:ci && yarn test:cy:ci",
"test:cy:ci": "yarn test:build && start-server-and-test start-test-server http://localhost:8080 cy:run",
"test:cy:watch": "start-server-and-test start-test-server http://localhost:8080 cy:open",
BrianSipple marked this conversation as resolved.
Show resolved Hide resolved
"test:cy:ci": "yarn test:build && start-server-and-test start-test-server http://localhost:9002 cy:run",
"test:cy:watch": "yarn test:build && start-server-and-test start-test-server http://localhost:9002 cy:open",
Copy link
Member

Choose a reason for hiding this comment

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

Do you think if we ran yarn watch here, instead of yarn test:build it would rebuild the assets? I think this was supposed to be live reloading, but not sure how it was supposed to work.

"test:unit:ci": "cross-env NODE_ENV=test webpack --config webpack.test.config.js && yarn mocha-headless-chrome",
"test:unit:watch": "webpack-dev-server --config webpack.test.config.js",
"watch": "yarn clean && webpack --watch --mode development"
Expand Down
2 changes: 1 addition & 1 deletion test/cypress/integration/test.acceptance.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ describe('Shepherd Acceptance Tests', () => {
beforeEach(() => {
Shepherd = null;

cy.visit('http://localhost:8080/test/dummy/', {
Copy link
Member

Choose a reason for hiding this comment

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

I think we had to manually set this here, rather than using baseURL to make sure that things worked for both the demo app and the test app.

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'm not sure I'm following. Do you mean running the demo app with yarn start? It doesn't seem like that would interfere with cy

Copy link
Member

Choose a reason for hiding this comment

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

There was some reason we needed the full path before. I don't remember why.

Copy link
Member

Choose a reason for hiding this comment

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

I think it was to support the demo app, what we deploy for the demo.

Copy link
Member

Choose a reason for hiding this comment

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

We can merge and see if the demo still looks good

cy.visit('/test/dummy/', {
onLoad(contentWindow) {
if (contentWindow.Shepherd) {
return Shepherd = contentWindow.Shepherd;
Expand Down