Improvement/cldsrv 724 multiple backend tests#5960
Conversation
1769d66 to
4619bab
Compare
❌ 366 Tests Failed:
View the top 3 failed test(s) by shortest run time
View the full list of 50 ❄️ flaky test(s)
To view more test analytics, go to the Test Analytics Dashboard |
76ab4cc to
fc9608c
Compare
ghost
left a comment
There was a problem hiding this comment.
Some quick comments as the pr was listed for review (but still a draft)
otherwise lgtm, some patterns could be simplified, when calling the next functions (by removing the {} or the =>)
e031412 to
1c923f3
Compare
a8406fa to
55803c6
Compare
307118e to
d6bfd4b
Compare
226f21d to
0cbf3b9
Compare
327e859 to
58f8c81
Compare
|
|
||
| after(async () => { | ||
| process.stdout.write('Emptying bucket\n'); | ||
| await bucketUtil.empty(bucket); |
There was a problem hiding this comment.
This is an addition right ? There was a flaky or something without it I guess ?
There was a problem hiding this comment.
yes , this is just to make sure we're emptying buckets before deleting them
| .then(() => { | ||
| assert.fail('Expected error but got success'); | ||
| }).catch(err => { | ||
| assert.strictEqual(err.code, 'NoSuchKey', 'Expected ' + |
There was a problem hiding this comment.
I'm surprised this err.code stays here without failing the pipeline, should it be err.name ?
There was a problem hiding this comment.
this pintpoints to gcp , we're returning codethat's why it is working
6d97621 to
d8c1d88
Compare
| assert.strictEqual(res.ServerSideEncryption, 'AES256'); | ||
| } | ||
|
|
||
| process.stdout.write('Getting object from AWS\n'); |
There was a problem hiding this comment.
| process.stdout.write('Getting object from AWS\n'); |
| }); | ||
| }); | ||
|
|
||
| // Create bucket using AWS SDK v3 |
There was a problem hiding this comment.
| // Create bucket using AWS SDK v3 |
| await s3.send(new PutObjectCommand({ Bucket: '', Key: key })); | ||
| throw new Error('Expected failure but got success'); | ||
| } catch (err) { | ||
| assert.strictEqual(err.code, 'MethodNotAllowed'); |
There was a problem hiding this comment.
should be err.name ?
Also line 105 there is a comment about v2.363, the comment is probably outdated
There was a problem hiding this comment.
ah the test is skipped, I guess you can change it or leave it, the pr is already good and it will be spotted if the test is used again
| await s3.send(new PutObjectCommand(params)); | ||
| throw new Error('Expected failure but got success'); | ||
| } catch (err) { | ||
| assert.strictEqual(err.code, 'InvalidArgument'); |
| done(); | ||
| }); | ||
|
|
||
| await s3.send(new PutObjectCommand(params)).then(() => { |
There was a problem hiding this comment.
weird mix of await with then/catch 🤔
0cbf3b9 to
172e889
Compare
d8c1d88 to
0b979a4
Compare
f0a4294 to
c244695
Compare
DarkIsDude
left a comment
There was a problem hiding this comment.
There is a lot of stuff that can be simplified with cb(err) but it's nit, so let's move forward. Congrats 🙏
| .then(() => { | ||
| assert.fail('Expected error but got success'); | ||
| }).catch(err => { | ||
| assert.strictEqual(err.code, 'NoSuchKey', 'Expected ' + |
| } | ||
|
|
||
| function _getAwsConfig(profile, config) { | ||
| const credentials = getAwsCredentials(profile, '/.aws/scality'); |
There was a problem hiding this comment.
because now the '/.aws/scality' is the default value used.
| assert.strictEqual(res.ServerSideEncryption, 'AES256'); | ||
| } | ||
|
|
||
| process.stdout.write('Getting object from AWS\n'); |
| await s3.send(new PutObjectCommand({ Bucket: '', Key: key })); | ||
| throw new Error('Expected failure but got success'); | ||
| } catch (err) { | ||
| assert.strictEqual(err.code, 'MethodNotAllowed'); |
172e889 to
ff86e9c
Compare
da7043b to
b74528b
Compare
Issue: CLDSRV-724