Skip to content

Commit

Permalink
fix: remove TEST_CACHE_NAME env var reference in tests (#1352)
Browse files Browse the repository at this point in the history
Previoulsy we read the test cache name from the environment in CI. We
no longer do this. However a developer may have TEST_CACHE_NAME
set.

In that case the integration tests will always use that for the cache
name. Because of this, control tests fail since some invoke
testCacheName() expecting a unique cache name as opposed to a
consistent one.

Speaking with other developers, they have been hit by this gotcha. One
solution would be to change tests to specify if they expect a unique
cache name vs some cache name. That will make the tests more difficult
to maintain, so we will remove the env var instead.
  • Loading branch information
malandis authored Jun 26, 2024
1 parent 095e04f commit 16ff0c5
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ export interface CacheClientPropsWithConfig extends CacheClientProps {
}

function testCacheName(): string {
return (
process.env.TEST_CACHE_NAME || `js-integration-test-compression-${v4()}`
);
return `js-integration-test-compression-${v4()}`;
}

let _credsProvider: CredentialProvider | undefined = undefined;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ export interface CacheClientPropsWithConfig extends CacheClientProps {
}

function testCacheName(): string {
return (
process.env.TEST_CACHE_NAME || `js-integration-test-compression-${v4()}`
);
return `js-integration-test-compression-${v4()}`;
}

let _credsProvider: CredentialProvider | undefined = undefined;
Expand Down
14 changes: 2 additions & 12 deletions packages/common-integration-tests/src/common-int-test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,11 @@ export function isLocalhostDevelopmentMode(): boolean {
return useLocalhost !== undefined;
}
export function testCacheName(): string {
return process.env.TEST_CACHE_NAME || `js-integration-test-default-${v4()}`;
return `js-integration-test-default-${v4()}`;
}

export function testStoreName(): string {
return process.env.TEST_STORE_NAME || `js-integration-test-default-${v4()}`;
}

/**
* Returns a unique index name for use in integration tests.
* @param meaningfulIdentifier Required suffix to identify the test for debugging purposes.
* Should be a string that describes the test uniquely.
* @returns {string} A unique index name.
*/
export function testIndexName(meaningfulIdentifier: string): string {
return `js-integration-test-${v4()}-${meaningfulIdentifier}`;
return `js-integration-test-default-${v4()}`;
}

export function testWebhook(cache?: string): Webhook {
Expand Down

0 comments on commit 16ff0c5

Please sign in to comment.