consolidate integration testing to platform dev account

Description

It seems the Py client integration tests use AWS account 294766370185. Please consolidate to the platform dev account so we can delete this account. (E.g., create a bucket in the dev' account for the test.) . Below is the content of an email exchange last August:

I'm going through the accounts to check that we're good on billing and came across 'https://294766370185.signin.aws.amazon.com/console' which I refer to as 'test'.
Looks like I set it up 3 years ago, it only contains S3 buckets that seem related to testing dev-pay/external S3 (good thing since it's on my card and not in consolidated billing...).
Only 2 IAM users, myself and 'travis-build' that only has access to 's3://test-client-auth-s3'. That bucket contains a few folders/files generated by tests last summer, however
IAM says 'travis-build' last accessed S3 on 7/27/2018. Any idea? I'll make the key inactive after break and delete the account if no complain.

Thx,
Xa

Hi Xa,

The Python client is using this bucket for its integration tests. Though I think the integration test can be rewritten as unit tests, since it exercises the same web services as other type of external storage upload location.
Question: are we still supporting this feature? If not, I would just remove the test instead of rewriting it. Otherwise, please either keep the bucket until we rewrite the test or please give me a different bucket to point the test to.

Thanks,
Kimyen

Environment

None

Activity

Show:
Kimyen Truong
November 9, 2018, 7:42 PM
Edited

Checking the current Python package configuration. We are using https://s3.amazonaws.com/test-client-auth-s3 in .synapseConfig. This is the test that is using it: https://github.com/Sage-Bionetworks/synapsePythonClient/blob/3b3155aa3c1aaa81ee561724e991d3caca8cf58e/tests/integration/integration_test.py#L468

The exception says:

This test only works on travis because it requires AWS credentials to a specific S3 bucket.

I think this test should be re-written as unit tests and removed. It's testing the following:

  • createStorageLocationSetting()

  • setStorageLocation()

  • store()

  • get()

It verifies that with a correct project setting, the file that is uploaded has the correct metadata. This logic should have been tested at PLFM level.

I will check to see if the above methods already has its unit tests. If so, we can remove this tests.

Kimyen Truong
November 9, 2018, 7:47 PM

I found that createStorageLocationSetting() and setStorageLocation() does not have any unit tests.

Kimyen Truong
December 13, 2018, 10:46 PM
Bruce Hoff
January 16, 2019, 6:25 PM

Validation: Checked the PR to see that the integration test is gone. Further, IT-380 will delete the AWS account that the old tests used.

Assignee

Kimyen Truong

Reporter

Bruce Hoff

Labels

Validator

Bruce Hoff

Development Area

None

Release Version History

None

Sprint

None

Fix versions

Priority

Major
Configure