Skip to content

Commit

Permalink
fix: add scenario 3 conformance tests (#1702)
Browse files Browse the repository at this point in the history
test: disable retries when creating write stream as part of upload.

test: add scenario 3 tests

update nodejs-common to 3.8.1

test: disable autoRetry when calling createWriteStream from save

linter fixes

fix test

fix unit tests by adding new string and additional guard case

add delay to ensure docker container is started
  • Loading branch information
ddelgrosso1 committed Nov 5, 2021
1 parent d265f8c commit e16a3a5
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 10 deletions.
16 changes: 12 additions & 4 deletions conformance-test/retryStrategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ function excecuteScenario(testCase: RetryTestCase) {
apiEndpoint: TESTBENCH_HOST,
projectId: CONF_TEST_PROJECT_ID,
});
creationResult = await createTestBenchRetryTest(
instructionSet.instructions,
jsonMethod?.name.toString()
);
});

beforeEach(async () => {
bucket = await createBucketForTest(
storage,
testCase.preconditionProvided,
Expand All @@ -137,13 +144,10 @@ function excecuteScenario(testCase: RetryTestCase) {
storageMethodString,
bucket
);

notification = bucket.notification(`${TESTS_PREFIX}`);
await notification.create();

creationResult = await createTestBenchRetryTest(
instructionSet.instructions,
jsonMethod?.name.toString()
);
storage.interceptors.push({
request: requestConfig => {
requestConfig.headers = requestConfig.headers || {};
Expand All @@ -155,6 +159,10 @@ function excecuteScenario(testCase: RetryTestCase) {
});
});

afterEach(() => {
storage.interceptors.pop();
});

it(`${storageMethodString}${instructionNumber}`, async () => {
if (testCase.expectSuccess) {
assert.ifError(
Expand Down
2 changes: 1 addition & 1 deletion conformance-test/test-bench-util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const DEFAULT_IMAGE_NAME =
const DEFAULT_IMAGE_TAG = 'latest';
const DOCKER_IMAGE = `${DEFAULT_IMAGE_NAME}:${DEFAULT_IMAGE_TAG}`;
const PULL_CMD = `docker pull ${DOCKER_IMAGE}`;
const RUN_CMD = `docker run --rm -d -p ${PORT}:${PORT} --name ${CONTAINER_NAME} ${DOCKER_IMAGE}`;
const RUN_CMD = `docker run --rm -d -p ${PORT}:${PORT} --name ${CONTAINER_NAME} ${DOCKER_IMAGE} && sleep 1`;
const STOP_CMD = `docker stop ${CONTAINER_NAME};`;

export async function getTestBenchDockerImage(): Promise<Buffer> {
Expand Down
21 changes: 18 additions & 3 deletions conformance-test/test-data/retryStrategyTestData.json
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,28 @@
},
{
"id": 3,
"description": "conditionally idempotent no retries when precondition is absent",
"description": "conditionally_idempotent_no_retries_when_precondition_is_absent",
"cases": [
{
"instructions": []
"instructions": ["return-503"]
},
{
"instructions": ["return-reset-connection"]
}
],
"methods": [],
"methods": [
{"name": "storage.buckets.patch", "resources": ["BUCKET"]},
{"name": "storage.buckets.setIamPolicy", "resources": ["BUCKET"]},
{"name": "storage.buckets.update", "resources": ["BUCKET"]},
{"name": "storage.hmacKey.update", "resources": ["HMAC_KEY"]},
{"name": "storage.objects.compose", "resources": ["BUCKET", "OBJECT"]},
{"name": "storage.objects.copy", "resources": ["BUCKET", "OBJECT"]},
{"name": "storage.objects.delete", "resources": ["BUCKET", "OBJECT"]},
{"name": "storage.objects.insert", "resources": ["BUCKET"]},
{"name": "storage.objects.patch", "resources": ["BUCKET", "OBJECT"]},
{"name": "storage.objects.rewrite", "resources": ["BUCKET", "OBJECT"]},
{"name": "storage.objects.update", "resources": ["BUCKET", "OBJECT"]}
],
"preconditionProvided": false,
"expectSuccess": false
},
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
"precompile": "gts clean"
},
"dependencies": {
"@google-cloud/common": "^3.7.4",
"@google-cloud/common": "^3.8.1",
"@google-cloud/paginator": "^3.0.0",
"@google-cloud/promisify": "^2.0.0",
"arrify": "^2.0.0",
Expand Down
6 changes: 6 additions & 0 deletions src/bucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3922,6 +3922,12 @@ class Bucket extends ServiceObject {
const returnValue = retry(
async (bail: (err: Error) => void) => {
await new Promise<void>((resolve, reject) => {
if (
numberOfRetries === 0 &&
newFile?.storage?.retryOptions?.autoRetry
) {
newFile.storage.retryOptions.autoRetry = false;
}
const writable = newFile.createWriteStream(options);
if (options.onUploadProgress) {
writable.on('progress', options.onUploadProgress);
Expand Down
11 changes: 10 additions & 1 deletion src/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3760,6 +3760,9 @@ class File extends ServiceObject<File> {
const returnValue = retry(
async (bail: (err: Error) => void) => {
await new Promise<void>((resolve, reject) => {
if (maxRetries === 0) {
this.storage.retryOptions.autoRetry = false;
}
const writable = this.createWriteStream(options)
.on('error', err => {
if (
Expand Down Expand Up @@ -4085,7 +4088,13 @@ class File extends ServiceObject<File> {
* that a callback is omitted.
*/
promisifyAll(File, {
exclude: ['publicUrl', 'request', 'save', 'setEncryptionKey'],
exclude: [
'publicUrl',
'request',
'save',
'setEncryptionKey',
'shouldRetryBasedOnPreconditionAndIdempotencyStrat',
],
});

/**
Expand Down
1 change: 1 addition & 0 deletions test/file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ const fakePromisify = {
'request',
'save',
'setEncryptionKey',
'shouldRetryBasedOnPreconditionAndIdempotencyStrat',
]);
},
};
Expand Down

0 comments on commit e16a3a5

Please sign in to comment.