async/await in tests

[43081j]

This cleans up tests a little on top of #808.

I noticed quite a few, if they failed, threw uncaught promises rather than ever reaching mocha so the errors weren't so useful.

Changing the async tests to use async/await instead of the done callback means uncaught promise exceptions will be handled by mocha. Makes it a bit cleaner to read too.

if you want to have a look at the diff, view the last commit by its self

[aoberoi]

this is a great idea. i think we really should have done this work back in #690 when we dropped support for versions that couldn't run async functions.

since then, we've moved to the monorepo structure and pulled in the @slack/interactive-messages and @slack/events-api codebases (as i know you know). this complicates things. these two packages were never aligned with the rest in terms of node version support - they currently are meant to support >= v4.2.0 (as shown in the package.json "engines" property). we would like to align the minimum node version support across the repo, but that means releasing a semver major change for those two packages. according to our published Support Schedule commitments, if we do that now, we still have to support the v1 and v2 branches of those packages until at least April 2020. however, the team feels like taking on the additional work of supporting two major versions isn't worth it just to get async functions into the tests.

so there's a middle road that i propose: can we move the tests for all the remaining packages to use async/await and leave the tests for those two packages alone?

PS. i recognize that we're living in a world where those packages are not being tested on CI against the node versions going back to v4.2.0. we should probably address that with #851.

[43081j]

technically we could still use it but have a different target for those packages (ES5 or some such thing), possibly? TS would transpile down to compatible syntax

otherwise don't mind reverting those particular packages

[aoberoi]

as of now, i'm assuming the tests will continue to be JavaScript, not TypeScript (as discussed in #808 (comment)). in this case, the target isn't relevant because there's no compilation step on the tests.

[43081j]

true. do both of these packages support async/await? (i.e. is the node version high enough?)

webhook and web-api

afaik the commit this PR is about contains only those two packages already anyway (just on top of my huge other PR, but could be separated)

[aoberoi]

oh yeah! both @slack/webhook and @slack/web-api have a minimum node version that is high enough to be able to use async functions 🎉

[codecov]

Codecov Report

Merging #810 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #810   +/-   ##
=======================================
  Coverage   95.62%   95.62%           
=======================================
  Files          12       12           
  Lines         686      686           
  Branches      147      147           
=======================================
  Hits          656      656           
  Misses         11       11           
  Partials       19       19

Flag	Coverage Δ
#eventsapi	89.61% <ø> (ø)	⬆️
#interactivemessages	96.33% <ø> (ø)	⬆️
#webapi	98.5% <ø> (ø)	⬆️
#webhook	87.5% <ø> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae18ead...ab841c2. Read the comment docs.

[43081j]

rebased and dropped the typescript.

build doesn't seem to have started but tests did pass locally

[43081j]

@aoberoi so i've updated to assert for non-assertionerror errors.

it still doesn't seem ideal but its simpler than adding complexity or new dependencies to have an async throws kinda pattern/assertion. some tests assert for a specific type of error so i left those as-is, too.