Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
samples(storage transfer): add samples and test cases for storage transfer services #2857
base: main
Are you sure you want to change the base?
samples(storage transfer): add samples and test cases for storage transfer services #2857
Changes from 28 commits
ba3c128
c2982ef
36706a4
afcba29
5d43c18
3efcf76
35e18a3
0acb7fd
e3c0619
000b432
910ad26
398b3d5
b5d9b10
3c9d243
f4b73e0
1acec24
375f1e0
81beca4
ce4a089
139190a
7357db6
bb66a63
526b445
8a06504
edc89e4
7d100d4
4b88480
effd0c2
f5d8252
be9c4bb
be74818
a69aaa1
4b5d2e5
52eb9de
f57ea40
bf89631
a2192fc
2d068d2
2e7aa62
02a0e14
5e70900
d942177
a59263c
7e2b4dd
04eeea7
332f78d
522b48a
8642fbf
7287261
904c989
1b0d4fa
dda9b52
f1b05b3
b05a6bc
526ddad
c6950d8
d043b9e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How many of these are being used by more than one test? In the fixture, we only should add resources that are used by more than one test. Otherwise, the test itself should create and clean up the resources.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JobName , SourceAgentPoolName , SinkAgentPoolName , RootDirectory are being used by more than one test
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resources for event driven transfer are moved from storage fixture to test itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amanda-tarafa Do I need to move directory paths (GcsSourcePath, DestinationDirectory, TempDirectory, TempDestinationDirectory) and ManifestObjectName property to respective tests from storage fixture ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If these are being used for a single test, and that inmediate expectation is that that remains the case, then yes, it's clearer to have them on their respective tests only. In the fixture you can have a method that helps with creating random IDs, file names, temporary paths etc. and you use those on the tests when initializing these values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amanda-tarafa just wanted to make sure once if there is any linter available which I by mistakenly missed out in documentation provided (apart from dot net format command in IDE) for .NET style and structure changes and more specific to blank spaces which I am manually doing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you use the Format command in Visual Studio you should be fine. The linter has been deactivated for some time because of reasons. We have #1080 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@amanda-tarafa I have created GenerateBucketName method in storage fixture similar to .NET client library and called the same in constructor of each individual tests from constructor injected fixture instance and also I made CreateBucketAndGrantStsPermissions internal from private and called same in constructor of each individual tests as it was being used by all tests and to avoid repeatative call of the same method in the fixture and cleaned up resources in individual test if created by test itself else cleaned up in storage fixture
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, thanks, that sounds good.