PrivateCoffee/element-web
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 3

Add flag for tests to avoid double-reporting check (#12569)

Browse source 
* Add flag for tests to avoid double-reporting check

Some of the tests were flaking.  Our best guess is that it's failing to track
some events due to false positives in the Bloom filter used to guard against
double-reporting.  So we add a flag to disable using that in tests (except for
tests that test that functionality).

* invert logic to avoid double negative
This commit is contained in:
Hubert Chathi 2024-06-06 09:02:34 -04:00 committed by GitHub
parent 39d0017411
commit c4c1faff97
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 23 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
src/DecryptionFailureTracker.ts
View file

@ -158,10 +158,16 @@ export class DecryptionFailureTracker {
*
* @param {function} errorCodeMapFn The function used to map decryption failure reason codes to the
* `trackedErrorCode`.
*
* @param {boolean} checkReportedEvents Check if we have already reported an event.
* Defaults to `true`. This is only used for tests, to avoid possible false positives from
* the Bloom filter. This should be set to `false` for all tests except for those
* that specifically test the `reportedEvents` functionality.
*/
private constructor(
private readonly fn: TrackingFn,
private readonly errorCodeMapFn: ErrCodeMapFn,
private readonly checkReportedEvents: boolean = true,
) {
if (!fn || typeof fn !== "function") {
throw new Error("DecryptionFailureTracker requires tracking function");
@ -214,7 +220,7 @@ export class DecryptionFailureTracker {
const eventId = e.getId()!;
// if it's already reported, we don't need to do anything
if (this.reportedEvents.has(eventId)) {
if (this.reportedEvents.has(eventId) && this.checkReportedEvents) {
return;
}
@ -240,7 +246,7 @@ export class DecryptionFailureTracker {
const eventId = e.getId()!;
// if it's already reported, we don't need to do anything
if (this.reportedEvents.has(eventId)) {
if (this.reportedEvents.has(eventId) && this.checkReportedEvents) {
return;
}

15
test/DecryptionFailureTracker-test.ts
View file

@ -52,6 +52,7 @@ describe("DecryptionFailureTracker", function () {
const tracker = new DecryptionFailureTracker(
() => count++,
() => "UnknownError",
false,
);
tracker.addVisibleEvent(failedDecryptionEvent);
@ -78,6 +79,7 @@ describe("DecryptionFailureTracker", function () {
reportedRawCode = rawCode;
},
() => "UnknownError",
false,
);
tracker.addVisibleEvent(failedDecryptionEvent);
@ -101,6 +103,7 @@ describe("DecryptionFailureTracker", function () {
const tracker = new DecryptionFailureTracker(
() => count++,
() => "UnknownError",
false,
);
eventDecrypted(tracker, failedDecryptionEvent, Date.now());
@ -121,6 +124,7 @@ describe("DecryptionFailureTracker", function () {
propertiesByErrorCode[errorCode] = properties;
},
(error: string) => error,
false,
);
// use three different errors so that we can distinguish the reports
@ -161,6 +165,7 @@ describe("DecryptionFailureTracker", function () {
expect(true).toBe(false);
},
() => "UnknownError",
false,
);
tracker.addVisibleEvent(decryptedEvent);
@ -189,6 +194,7 @@ describe("DecryptionFailureTracker", function () {
expect(true).toBe(false);
},
() => "UnknownError",
false,
);
eventDecrypted(tracker, decryptedEvent, Date.now());
@ -216,6 +222,7 @@ describe("DecryptionFailureTracker", function () {
const tracker = new DecryptionFailureTracker(
() => count++,
() => "UnknownError",
false,
);
tracker.addVisibleEvent(decryptedEvent);
@ -379,6 +386,7 @@ describe("DecryptionFailureTracker", function () {
(errorCode: string) => (counts[errorCode] = (counts[errorCode] || 0) + 1),
(error: DecryptionFailureCode) =>
error === DecryptionFailureCode.UNKNOWN_ERROR ? "UnknownError" : "OlmKeysNotSentError",
false,
);
const decryptedEvent1 = await createFailedDecryptionEvent({
@ -416,6 +424,7 @@ describe("DecryptionFailureTracker", function () {
const tracker = new DecryptionFailureTracker(
(errorCode: string) => (counts[errorCode] = (counts[errorCode] || 0) + 1),
(_errorCode: string) => "OlmUnspecifiedError",
false,
);
const decryptedEvent1 = await createFailedDecryptionEvent({
@ -450,6 +459,7 @@ describe("DecryptionFailureTracker", function () {
const tracker = new DecryptionFailureTracker(
(errorCode: string) => (counts[errorCode] = (counts[errorCode] || 0) + 1),
(errorCode: string) => Array.from(errorCode).reverse().join(""),
false,
);
const decryptedEvent = await createFailedDecryptionEvent({
@ -475,6 +485,7 @@ describe("DecryptionFailureTracker", function () {
},
// @ts-ignore access to private member
DecryptionFailureTracker.instance.errorCodeMapFn,
false,
);
const now = Date.now();
@ -543,6 +554,7 @@ describe("DecryptionFailureTracker", function () {
propertiesByErrorCode[errorCode] = properties;
},
(error: string) => error,
false,
);
// use three different errors so that we can distinguish the reports
@ -597,6 +609,7 @@ describe("DecryptionFailureTracker", function () {
errorCount++;
},
(error: string) => error,
false,
);
// Calling .start will start some intervals. This test shouldn't run
@ -638,6 +651,7 @@ describe("DecryptionFailureTracker", function () {
propertiesByErrorCode[errorCode] = properties;
},
(error: string) => error,
false,
);
// @ts-ignore access to private method
@ -710,6 +724,7 @@ describe("DecryptionFailureTracker", function () {
failure = properties;
},
() => "UnknownError",
false,
);
tracker.addVisibleEvent(failedDecryptionEvent);