Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Update the integration tests to support use against a remote host/server. #154

Closed
jbonacci opened this issue Aug 21, 2013 · 14 comments
Closed
Assignees

Comments

@jbonacci
Copy link

I would like to be able to run our tests and hit our Dev server in API.

Short of hacking the following files
https://github.com/mozilla/picl-idp/blob/master/config/config.js
https://github.com/mozilla/picl-idp/blob/master/test/run/integration_tests.js

it would be helpful to support the following:
ENV for the default public url similar to what was done for the KVSTORE* variables in config.js
So I can do something like this:
TEST_REMOTE=http://idp.dev.lcip.org:9000 npm test

randomly generated restmail accounts in integration_tests.js
(vs. using fixed example.com emails)
So I can run the tests more than once w/o failing on the account creation stuff
Alternately, we could have a script the wipes the db of all example.com emails

Thoughts?
@dannycoates
@zaach
@rfk

Related issues that could benefit:
#81
#94

@ghost ghost assigned dannycoates Aug 22, 2013
@dannycoates
Copy link
Contributor

@jbonacci can you describe more about how running the integration tests against a remote machine is useful? What is the intent?

From my perspective (having not thought much about it) the integration tests are good for running locally as a dev to catch logic errors. For more thorough testing: load, concurrency, stress, security, fuzzing, etc, we need more sophisticated frameworks like mozilla-services/loads and other specialized tests. Given that I don't understand the value of running the existing integration tests remotely.

I'm not saying I disagree with your issue, I just want to understand more about it.

FYI, I'm working on making the integration tests easier to run locally with mysql, including cleaning the db here dannycoates@16c8c94 which should be useful for remote testing also.

@jbonacci
Copy link
Author

@dannycoates
What you said above may be enough.
Running against a remote server is a bridge (generalized testing against a remote host) until we get the full loads testing set up...

@rfk
Copy link
Contributor

rfk commented Aug 26, 2013

Running against a remote server is a bridge until we get the full loads testing set up...

Weeell...yes and no. Loads testing is designed to hit the most frequently-accessed endpoints with great load, but it's not going to catch a lot of corner-case API problems. Unless we duplicate the integration tests into the loadtest suite, which seems like a waste.

Ideally I'd like to have an "API test suite" that just runs through and exercises as much of the API as possible, including error conditions and corner cases, that can be run against a live server. We could run it to sanity-check the deployment of a new stack, and we could run it while the system it under load to ensure that none of the corner-cases start misbehaving. Just last week we had a mysteriously deployment problem in sync1.1 whose debugging would have been helped by an easily-runnable API test suite.

If we can re-use the integration tests to create such a thing, then IMO we get a lot of extra value for free. If it's cleaner to create a separate suite of live-API-sanity-checking-tests then that's OK too, but we should then include it back into the regularly-run dev test suite.

@ckarlof
Copy link
Contributor

ckarlof commented Aug 27, 2013

Ideally I'd like to have an "API test suite" that just runs through and exercises as much of the API as possible, including error conditions and corner cases, that can be run against a live server. We could run it to sanity-check the deployment of a new stack, and we could run it while the system it under load to ensure that none of the corner-cases start misbehaving.

In the past, I've gotten value out of integration tests I can run against a remote server for these reasons.

@jbonacci
Copy link
Author

OK. So, let's evaluate the possibility of updating/upgrading/re-engineering the integration tests for this purpose. If that is too much work, or strays from the intention of the integration suite, then let's close this issue and open one that is more specific to an API test suite.

@dannycoates does that sound ok with you?
@rfk either way, can you work with me on this?

@edwindotcom
Copy link

+1 on API Test Suite

@dannycoates
Copy link
Contributor

@jbonacci yeah, the integration_tests.js are ready to run against a remote server in my fork.

The verification_tests.js are the tricky ones because of email. The local only version is super simple, yet annoyingly more complicated to work well with restmail.

@rfk I guess my assumption earlier was that the load tests would exercise the full API. Given your response I agree we should get to a nice API test suite, but I don't have the cycles to work on it yet. Local tests are useful enough for me at this point. Once I'm confident enough in the code to hammer it I'll start writing another round of tests.

@rfk
Copy link
Contributor

rfk commented Aug 29, 2013

Per your suggestion I tweaked the existing integration_tests.js and successfully used it for this purpose: #171

A very nice start! :-)

@jbonacci
Copy link
Author

Here is what I see for the following:
PUBLIC_URL="http://idp.dev.lcip.org" node test/run/integration_tests.js
This fails - I assume this is expected: http://jbonacci.pastebin.mozilla.org/3018820
(Note: I npm installed picl-idp, generated the keys, and ran the line above. I did not do the usual "npm start")
Let me know if these results are expected.

I can not test this idea against the Dev loadtest environment since it appears to be down.
Will retest once @rfk and I set that up again.

@jbonacci
Copy link
Author

Errors mentioned above against Dev environment are NOT expected.
I just opened 193...

@jbonacci
Copy link
Author

POC Verified:
PUBLIC_URL="https://idp.dev.lcip.org" node test/run/integration_tests.js

Leaving this issue open since there might be more work that can be done...

@jbonacci
Copy link
Author

@edmoz and @pdehaan
Take a look at this issue in context with your ideas for additional integration and API tests.

@jbonacci
Copy link
Author

This is still an open issue. I am hooking it to #264 since any remote host we use will most likely be in AWS...

@jbonacci
Copy link
Author

Silly me - Dev and Dev-Latest are in AWS.

vladikoff pushed a commit that referenced this issue Feb 17, 2017
* Password changed template shows "reset your password" link.
  "change your password" used to be displayed. If an attacker changed
  the user's password, the user would not know the new password to change it.
* Fix the "reset password" link color in the "Your password has been reset" email.
* All password reset links auto-submit the password reset form.
* The "Change your password" link in the "New sign in to Firefox" email
  contains the users email address.

fixes #151
fixes #153
fixes #154
fixes #155
fixes #157
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants