-
Notifications
You must be signed in to change notification settings - Fork 34
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
Feat/dataprotection -- Tests & Docs #280
Feat/dataprotection -- Tests & Docs #280
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## feat/dataprotection #280 +/- ##
=======================================================
- Coverage 25.41% 25.40% -0.01%
=======================================================
Files 155 155
Lines 13601 13605 +4
=======================================================
Hits 3457 3457
- Misses 9987 9991 +4
Partials 157 157
☔ View full report in Codecov by Sentry. |
@GilTeraSky, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
c1cb319
to
f75f923
Compare
@GilTeraSky, you must sign every commit in this pull request acknowledging our Developer Certificate of Origin before your changes are merged. This can be done by adding
|
f75f923
to
50b013f
Compare
8b16489
to
d5a88cf
Compare
a855357
to
ab6144f
Compare
a8e8301
to
802e37a
Compare
ab6144f
to
d698e14
Compare
Signed-off-by: GilTS <[email protected]>
67b3081
to
24fe64a
Compare
Signed-off-by: GilTS <[email protected]>
Signed-off-by: GilTS <[email protected]>
Signed-off-by: GilTS <[email protected]>
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.
LGTM
Signed-off-by: GilTS <[email protected]>
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.
@GilTeraSky - Add mock tests too for data protection feature, you can refer akscluster resource, but you can take it up after utkg work
@@ -0,0 +1,16 @@ | |||
terraform { | |||
required_providers { |
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.
Is there a usage for this file?
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.
I put this in the folder so it will be easy to test from this folder, should I remove it?
We already agreed that with the Converter there is no need to mock anything and we should add tests for the converter in the future by me or someone else. |
When I say mock test, it Is mimicking the server with custom request/response so that we can cover even edge cases which we usually can't cover with the actual server. let me know if need more details |
I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
What this PR does / why we need it
This PR is the final in the series of Data Protection resources task.
It includes tests and docs for data protection enablement, target location and backup schedule resources and data sources.
Which issue(s) this PR fixes
Additional information
Special notes for your reviewer
For running the acceptance tests it is required to have 2 environment variables:
TMC_MANAGED_CREDENTIALS_NAME - this variable should be a name of a TMC managed credentials which is required for
running the tests. Although it is possible to create a TMC managed credentials via the resource, there is a necessity for the
credentials to include a valid IAM role because when creating a target location with this type of credentials it fails if the IAM
role doesn't have permissions.
AZURE_CREDENTIALS_NAME - this variable should be a name of a azure self provisioned credentials which is required for
running the tests. It is not supported creating azure self provisioned credentials for data protection use case.