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

allow to configure proxy via env var #761

Merged
merged 6 commits into from Dec 23, 2018

Conversation

2 participants
@omerlh
Copy link
Contributor

omerlh commented Dec 16, 2018

As part of a session I'm doing, I wanted to demo Zap capabilities as DAST solution. The best test case is, of course, juice shop. So I added a small change that will allow to run the existing e2e tests and scan them with Zap. All that is now required is to run:

http_proxy=http://localhost:8080 yarn run e2e

And the e2e will be proxied via Zap (assuming http://localhost:8080 is Zap's proxy endpoint).
Is this something that you would like to merge? Do you want me to add a small docker compose file that will run everything in compose?
BTW I tried to proxy the API tests, but I couldn't find how to control firsby proxy behavior. I managed to do it only by changing the tests code, but this is pretty ugly and not something I think should merged.
Soluto/webdriverio-zap-proxy/issues/6

@bkimminich

This comment has been minimized.

Copy link
Owner

bkimminich commented Dec 16, 2018

I like the idea, but http_proxy is kind of a standard system variable name, that it might be set to e.g. some corporate proxy - and then this would mess with the tests, right?

@omerlh

This comment has been minimized.

Copy link
Contributor

omerlh commented Dec 17, 2018

That's a tricky question. I choose http_proxy because it's so common, so I thought it's better to use the regular environment variable that controls proxy settings.
In the case of a corporate proxy - it just means that the tests will run using this proxy - which might be the expected behavior. E.g. - sometime the proxy is the only way to reach the internet.

omerlh added some commits Dec 17, 2018

@omerlh

This comment has been minimized.

Copy link
Contributor

omerlh commented Dec 22, 2018

Ok, now the tests should finally pass.

@omerlh

This comment has been minimized.

Copy link
Contributor

omerlh commented Dec 22, 2018

Do you know why travis test failed although it look like a success?

Protractor exited with code 0 (SUCCESS)
The command "node_version=$(node -v); if [ ${node_version:1:2} = 10 ]; then NODE_ENV=ctf npm run protractor; else echo "Skipping Protractor e2e tests on $node_version"; fi" exited with 0.
@bkimminich

This comment has been minimized.

Copy link
Owner

bkimminich commented Dec 22, 2018

It failed already at running the code style checks:


standard: Use JavaScript Standard Style (https://standardjs.com)
standard: Run `standard --fix` to automatically fix some problems.
/home/travis/build/bkimminich/juice-shop/protractor.conf.js:5:2: Extra semicolon.
/home/travis/build/bkimminich/juice-shop/protractor.conf.js:7:28: Expected '!==' and instead saw '!='.
npm ERR! Test failed. See above for more details.
The command "npm test" exited with 1.

Travis-CI runs all tests regardsless of prior suites failing. That can be confusing to spot the actual failure, but it avoids chasing from one failing suite to the next.

@omerlh

This comment has been minimized.

Copy link
Contributor

omerlh commented Dec 23, 2018

Ohh yes, I missed that. Thanks! Hope now it will pass...

@omerlh

This comment has been minimized.

Copy link
Contributor

omerlh commented Dec 23, 2018

Now it finally passed, let me know if you want me to change the name of the env var :)

@bkimminich

This comment has been minimized.

Copy link
Owner

bkimminich commented Dec 23, 2018

Looks good, will try the behavior in proxied corp. environment after merging! 👍

If you want to try your luck with API tests, here might be a solution: vlucas/frisby#363

@bkimminich bkimminich changed the base branch from master to develop Dec 23, 2018

@bkimminich bkimminich merged commit 9b9d28e into bkimminich:develop Dec 23, 2018

5 checks passed

codeclimate All good!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
security/snyk - frontend/package.json (bkimminich) No new issues
Details
security/snyk - package.json (bkimminich) No new issues
Details

bkimminich added a commit to bkimminich/pwning-juice-shop that referenced this pull request Dec 23, 2018

@omerlh

This comment has been minimized.

Copy link
Contributor

omerlh commented Dec 23, 2018

By looking on the issue, we either need to that per file, or create frisby in one file, and reuse the same instance in all tests. Am I right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment