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

fix(tests): pick up latest auth db in auth server tests #796

Merged
merged 2 commits into from Apr 11, 2019

Conversation

Projects
None yet
2 participants
@philbooth
Copy link
Member

commented Apr 10, 2019

Fixes #793.

This is what I had in mind to hook the auth server tests up with the auth db from this repo. When Circle is back I'll restart the build to see how it gets on.

@philbooth philbooth self-assigned this Apr 10, 2019

@philbooth philbooth force-pushed the pb/793-auth-server-db branch 11 times, most recently from 31d6b91 to 5760e1e Apr 11, 2019

@philbooth

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2019

This got close to where I wanted it to be, but some remote tests are failing and at least some of those failures look like they're related to the same unicode MySQL stuff I suffered with in the email service (ref #782):

  299 passing (5m)
  10 failing

  1) remote account create
       account creation works with unicode email address:
     An internal server error occurred
  

  2) remote account login
       login works with unicode email address:
     An internal server error occurred
  

  3) remote account profile
       account profile works with unicode email address:
     An internal server error occurred
  

  4) remote account status
       account status by email works with unicode email address:
     An internal server error occurred
  

  5) remote base path
       "/" returns valid version information:
     AssertionError: source repository: expected false to be truthy
      at request.spread (test/remote/base_path_tests.js:34:18)
      at tryCatcher (node_modules/bluebird/js/release/util.js:16:23)
      at Promise._settlePromiseFromHandler (node_modules/bluebird/js/release/promise.js:509:35)
      at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:569:18)
      at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
      at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:694:18)
      at Promise._fulfill (node_modules/bluebird/js/release/promise.js:638:18)
      at PromiseArray._resolve (node_modules/bluebird/js/release/promise_array.js:126:19)
      at PromiseArray._promiseFulfilled (node_modules/bluebird/js/release/promise_array.js:144:14)
      at PromiseArray._iterate (node_modules/bluebird/js/release/promise_array.js:114:31)
      at PromiseArray.init [as _init] (node_modules/bluebird/js/release/promise_array.js:78:10)
      at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:566:21)
      at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
      at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:694:18)
      at Promise._fulfill (node_modules/bluebird/js/release/promise.js:638:18)
      at Request._callback (node_modules/bluebird/js/release/nodeback.js:45:21)
      at Request.self.callback (node_modules/request/request.js:185:22)
      at Request.EventEmitter.emit (domain.js:441:20)
      at Request.<anonymous> (node_modules/request/request.js:1161:10)
      at Request.EventEmitter.emit (domain.js:441:20)
      at IncomingMessage.<anonymous> (node_modules/request/request.js:1083:12)
      at IncomingMessage.EventEmitter.emit (domain.js:441:20)
      at endReadableNT (_stream_readable.js:1125:12)
      at process._tickCallback (internal/process/next_tick.js:63:19)

  6) remote base path
       "/__version__" returns valid version information:
     AssertionError: source repository: expected false to be truthy
      at request.spread (test/remote/base_path_tests.js:34:18)
      at tryCatcher (node_modules/bluebird/js/release/util.js:16:23)
      at Promise._settlePromiseFromHandler (node_modules/bluebird/js/release/promise.js:509:35)
      at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:569:18)
      at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
      at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:694:18)
      at Promise._fulfill (node_modules/bluebird/js/release/promise.js:638:18)
      at PromiseArray._resolve (node_modules/bluebird/js/release/promise_array.js:126:19)
      at PromiseArray._promiseFulfilled (node_modules/bluebird/js/release/promise_array.js:144:14)
      at PromiseArray._iterate (node_modules/bluebird/js/release/promise_array.js:114:31)
      at PromiseArray.init [as _init] (node_modules/bluebird/js/release/promise_array.js:78:10)
      at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:566:21)
      at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
      at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:694:18)
      at Promise._fulfill (node_modules/bluebird/js/release/promise.js:638:18)
      at Request._callback (node_modules/bluebird/js/release/nodeback.js:45:21)
      at Request.self.callback (node_modules/request/request.js:185:22)
      at Request.EventEmitter.emit (domain.js:441:20)
      at Request.<anonymous> (node_modules/request/request.js:1161:10)
      at Request.EventEmitter.emit (domain.js:441:20)
      at IncomingMessage.<anonymous> (node_modules/request/request.js:1083:12)
      at IncomingMessage.EventEmitter.emit (domain.js:441:20)
      at endReadableNT (_stream_readable.js:1125:12)
      at process._tickCallback (internal/process/next_tick.js:63:19)

  7) remote email validity
       /account/create with a variety of unusual but valid email addresses:
     AssertionError: Email address Ӓ@example.com should have been allowed, but it wasn't
      at emails.(anonymous function).Client.create.then (test/remote/email_validity_tests.js:76:15)
      at tryCatcher (node_modules/bluebird/js/release/util.js:16:23)
      at Promise._settlePromiseFromHandler (node_modules/bluebird/js/release/promise.js:512:31)
      at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:569:18)
      at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
      at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:690:18)
      at _drainQueueStep (node_modules/bluebird/js/release/async.js:138:12)
      at _drainQueue (node_modules/bluebird/js/release/async.js:131:9)
      at Async._drainQueues (node_modules/bluebird/js/release/async.js:147:5)
      at Immediate.Async.drainQueues [as _onImmediate] (node_modules/bluebird/js/release/async.js:17:14)
      at process.topLevelDomainCallback (domain.js:120:23)

  8) remote misc
       / returns version, git hash and source repo:
     AssertionError: source repository: expected false to be truthy
      at request.spread (test/remote/misc_tests.js:32:16)
      at tryCatcher (node_modules/bluebird/js/release/util.js:16:23)
      at Promise._settlePromiseFromHandler (node_modules/bluebird/js/release/promise.js:509:35)
      at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:569:18)
      at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
      at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:694:18)
      at Promise._fulfill (node_modules/bluebird/js/release/promise.js:638:18)
      at PromiseArray._resolve (node_modules/bluebird/js/release/promise_array.js:126:19)
      at PromiseArray._promiseFulfilled (node_modules/bluebird/js/release/promise_array.js:144:14)
      at PromiseArray._iterate (node_modules/bluebird/js/release/promise_array.js:114:31)
      at PromiseArray.init [as _init] (node_modules/bluebird/js/release/promise_array.js:78:10)
      at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:566:21)
      at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
      at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:694:18)
      at Promise._fulfill (node_modules/bluebird/js/release/promise.js:638:18)
      at Request._callback (node_modules/bluebird/js/release/nodeback.js:45:21)
      at Request.self.callback (node_modules/request/request.js:185:22)
      at Request.EventEmitter.emit (domain.js:441:20)
      at Request.<anonymous> (node_modules/request/request.js:1161:10)
      at Request.EventEmitter.emit (domain.js:441:20)
      at IncomingMessage.<anonymous> (node_modules/request/request.js:1083:12)
      at IncomingMessage.EventEmitter.emit (domain.js:441:20)
      at endReadableNT (_stream_readable.js:1125:12)
      at process._tickCallback (internal/process/next_tick.js:63:19)

  9) remote misc
       /__version__ returns version, git hash and source repo:
     AssertionError: source repository: expected false to be truthy
      at request.spread (test/remote/misc_tests.js:32:16)
      at tryCatcher (node_modules/bluebird/js/release/util.js:16:23)
      at Promise._settlePromiseFromHandler (node_modules/bluebird/js/release/promise.js:509:35)
      at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:569:18)
      at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
      at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:694:18)
      at Promise._fulfill (node_modules/bluebird/js/release/promise.js:638:18)
      at PromiseArray._resolve (node_modules/bluebird/js/release/promise_array.js:126:19)
      at PromiseArray._promiseFulfilled (node_modules/bluebird/js/release/promise_array.js:144:14)
      at PromiseArray._iterate (node_modules/bluebird/js/release/promise_array.js:114:31)
      at PromiseArray.init [as _init] (node_modules/bluebird/js/release/promise_array.js:78:10)
      at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:566:21)
      at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
      at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:694:18)
      at Promise._fulfill (node_modules/bluebird/js/release/promise.js:638:18)
      at Request._callback (node_modules/bluebird/js/release/nodeback.js:45:21)
      at Request.self.callback (node_modules/request/request.js:185:22)
      at Request.EventEmitter.emit (domain.js:441:20)
      at Request.<anonymous> (node_modules/request/request.js:1161:10)
      at Request.EventEmitter.emit (domain.js:441:20)
      at IncomingMessage.<anonymous> (node_modules/request/request.js:1083:12)
      at IncomingMessage.EventEmitter.emit (domain.js:441:20)
      at endReadableNT (_stream_readable.js:1125:12)
      at process._tickCallback (internal/process/next_tick.js:63:19)

  10) remote password forgot
       forgot password:
     AssertionError: flow begin time set: expected undefined to equal 1554983557621
      at Client.createAndVerify.then.then.then.then.then.then.then (test/remote/password_forgot_tests.js:87:20)
      at tryCatcher (node_modules/bluebird/js/release/util.js:16:23)
      at Promise._settlePromiseFromHandler (node_modules/bluebird/js/release/promise.js:512:31)
      at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:569:18)
      at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:614:10)
      at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:694:18)
      at _drainQueueStep (node_modules/bluebird/js/release/async.js:138:12)
      at _drainQueue (node_modules/bluebird/js/release/async.js:131:9)
      at Async._drainQueues (node_modules/bluebird/js/release/async.js:147:5)
      at Immediate.Async.drainQueues [as _onImmediate] (node_modules/bluebird/js/release/async.js:17:14)
      at process.topLevelDomainCallback (domain.js:120:23)

Obviously we can't just delete the failing tests here like I did for the email service, so I'm tweaking the approach to make sure it still uses Dockerflow-test like it did before. If it works that might be the correct path to take in #782 as well.

@philbooth philbooth force-pushed the pb/793-auth-server-db branch 3 times, most recently from 5ec0c1e to b00485e Apr 11, 2019

@philbooth philbooth requested a review from mozilla/fxa-devs Apr 11, 2019

@philbooth

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2019

Okay, I think this is working now.

The approach I've taken is to require a local copy of fxa-auth-db-mysql in the tests, then make sure that is available in most contexts:

  • For local dev, _scripts/install_all.sh copies it so it should be there for anyone who sets up the monorepo from scratch. (and I took the liberty of doing something similar for fxa-email-service)

  • There's also a belt-and-braces check in fxa-auth-server/scripts/test-local.sh that copies it if it can, but is non-fatal if run in a context where it doesn't exist to copy from. This is so that it works for people who haven't done an npm i from the monorepo root yet.

  • For CI, it's been added to fxa-auth-server/Dockerfile-test with a commensurate step added to .circleci/build.sh that makes the copy available when building the Docker image.

However, there is nothing that copies the directory for fxa-dev. I didn't realise before but those images don't even have the dev dependencies (sorry @vbudhram, you did try to point this out to me in #778 (comment), but I didn't realise the difference between the two Dockerfiles at that point).

The core logic for copying/running the auth db has been pulled out to _scripts/clone-authdb.sh since it is used in so many places.

@mozilla/fxa-devs r?

@philbooth

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2019

@mozilla/fxa-devs, this PR always uses the master branch for the copy of fxa-auth-db-mysql. Would it be better to use the current HEAD instead? That should mean it works for pull requests and master and CI...

@philbooth

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2019

Would it be better to use the current HEAD instead?

Fwiw I think that makes heaps more sense, plus it also avoids all that silly git sparse-checkout malarkey I had going on in the script because it can just copy from the local file system now. Not sure why I didn't just start off like that tbh. 😕

Anyway, have pushed with that change.

@vbudhram
Copy link
Contributor

left a comment

@philbooth No issues from me with the approach here, r+!

@@ -12,8 +12,8 @@
},
{
"name": "auth-server db memory PORT 8000",
"script": "node_modules/fxa-auth-db-mysql/bin/mem.js",

This comment has been minimized.

Copy link
@vbudhram

vbudhram Apr 11, 2019

Contributor

Nice catch! Completely forgot about this.

@vbudhram

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2019

Would it be better to use the current HEAD instead?

Long term I think we should do that, but probably ok to get this in and iterate scripts.

@philbooth philbooth merged commit a954c6e into master Apr 11, 2019

1 check passed

test Workflow: test
Details

@philbooth philbooth deleted the pb/793-auth-server-db branch Apr 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.