Conversation
rmi22186
left a comment
rmi22186
left a comment
There was a problem hiding this comment.
mostly lgtm. tiny nits
test/src/tests.js
Outdated
| this.createLauncherCalled = false; | ||
| this.createLauncher = function (options) { | ||
| self.accountId = options.accountId; | ||
| self.sandbox = options.sandbox; |
There was a problem hiding this comment.
note that we will remove this in a future PR and use the isDevelopmentMode flag. this will make it a bit harder to test since we'd have to init the mParticle SDK, although we could just mock it and simplify it a ton.
test/src/tests.js
Outdated
| // window.MockXYZForwarder.userId.should.equal('123'); | ||
|
|
||
| done(); | ||
| const waitForCondition = function async( |
There was a problem hiding this comment.
i'd bring this higher up in the file. feels weird to have it at the bottom.
| var moduleId = 181; | ||
|
|
||
| function attachLauncher(accountId, sandboxMode) { | ||
| window.Rokt.createLauncher({ |
There was a problem hiding this comment.
Minor, just wanted to check if we could start using async/await?
There was a problem hiding this comment.
We can't really use async/await because this file isn't being transpiled and we need kits to be as es5 as possible.
There was a problem hiding this comment.
^^ this. technically even promises aren't ES5, but since your customer base is all.
As an FYI to Ryan, we've had to support ES5 primarily because some media customers use our web SDK on very old devices, like smart TVs from early 2000s. Theoretically they'd probably not be using Rokt on those devices, but for now let's do it this way until we get kits properly transpiled.
alexs-mparticle
force-pushed
the
fix/SQDSDKS-7169-add-tests
branch
from
011d7ee to
00c12df
Compare
3 participants
Summary
Testing Plan