test(codecatalyst): only fail test if message is more than a minute off. #6265
Conversation
|
/runIntegrationTests |
| assert.ok( | ||
| Math.abs(actualMessages[i].minute - expected.minute) <= 1, | ||
| `Expected to be within 60 seconds of minute ${expected.minute}, but instead was at minute ${actualMessages[i].minute}` | ||
| ) | ||
| assert.deepStrictEqual(actualMessages[i], expected) |
There was a problem hiding this comment.
should drop this line, right?
There was a problem hiding this comment.
I was intending this line to check if the time the message popped up was within a minute of the expected time. Do you think it would be better to not check the time the message came up at all?
There was a problem hiding this comment.
If we don't remove "something", the test will still fail, no?
There was a problem hiding this comment.
I'm making the check more lenient. The logs in the issue show that the message is popping up at minute 54, when we expect it at 55. If that happens, the current logic should pass now since its within one minute of the expected time.
There was a problem hiding this comment.
If I understand correctly he's confused about why we aren't removing
assert.deepStrictEqual(actualMessages[i], expected)
since
assert.ok(Math.abs(actualMessages[i].minute - expected.minute) <= 1, 'Expected to be within 60 seconds of minute ${expected.minute}, but instead was at minute ${actualMessages[i].minute}')
is meant to replace it. If we kept the original check (assert.deepStrictEqual(actualMessages[i], expected)), wouldn't that check still fail since that piece of code is also checking the time?
There was a problem hiding this comment.
Oh my bad. Yes, we should remove that line. Sorry was looking at the wrong line and totally missed it.
## Problem fix aws#6213 ## Solution - The important part of this test is that the message comes after being inactive. If the message comes slightly early/late, that is okay. If it doesn't come at all, then we have problems and should fail. - Pass test if message comes within a minute of expected.
Problem
fix #6213
Solution
Alternatives
feature/xbranches will not be squash-merged at release time.License: I confirm that my contribution is made under the terms of the Apache 2.0 license.