inspectConfig added to jobTrigger #7677
Conversation
modular-magician
added
the
awaiting-approval
|
@slevenick Can you start the tests? It is awaiting approval from your end, Thanks. |
modular-magician
removed
the
awaiting-approval
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 4 files changed, 4153 insertions(+), 201 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: Please add acceptance tests which include these fields. |
Tests analyticsTotal tests: Errors occurred during REPLAYING mode. Please fix them to complete your PR |
|
Is inclusion of all the added fields in tests is necessary? If that's the case, will add a few more tests which cover all the attributes. Also, I've noticed some fields which I've added to tests are also showed missing. For example, |
| } | ||
|
|
||
| VcrTest(t, resource.TestCase{ | ||
| PreCheck: func() { testAccPreCheck(t) }, |
There was a problem hiding this comment.
This function has to be AccTestPreCheck
There was a problem hiding this comment.
I've updated the value. And to me, it seems like PreCheck used to fail and gave error for missing tests for certain attributes.
It is recommended to include all fields in tests. If the test has been added for a field, you can ignore the note for this field in the Missing test report. |
modular-magician
added
the
awaiting-approval
…ob-trigger-missing-attr
…ob-trigger-missing-attr
|
@zli82016 I've added extra tests and done the necessary changes. Can you please start the tests? |
modular-magician
removed
the
awaiting-approval
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 4 files changed, 4267 insertions(+), 337 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: Please add acceptance tests which include these fields. |
| - !ruby/object:Api::Type::Boolean | ||
| name: 'excludeInfoTypes' | ||
| description: When true, excludes type information of the findings. | ||
| - !ruby/object:Api::Type::Boolean |
There was a problem hiding this comment.
To send the false value, send_empty_value: true is needed for this field.
There was a problem hiding this comment.
Isn't this field a regular boolean value which can be sent true/false without send_empty_value: true? I found the same field in resource dlp_inspect_template in a similar way here. Similar to above declared attribute excludeInfoTypes .
mmv1/products/dlp/JobTrigger.yaml
Outdated
| name: 'maxFindingsPerItem' | ||
| description: Max number of findings that will be returned for each item | ||
| scanned. The maximum returned is 2000. | ||
| required: true |
There was a problem hiding this comment.
Just want to double check if this field is required.
There was a problem hiding this comment.
I checked the field manually and it isn't required, same goes for all other fields in limits. Hence, I've added AtleastOneOf instead of keeping all of them required.
Thanks so much for doing the check and making the changes. |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 4 files changed, 4792 insertions(+), 338 deletions(-)) Missing test reportYour PR includes resource fields which are not covered by any test. Resource: Please add acceptance tests which include these fields. |
|
I've added the missing fields, still it's being shown in the |
Tests analyticsTotal tests: Action takenFound 8 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccDataLossPreventionJobTrigger_dlpJobTriggerInspectCustomInfoTypes|TestAccApigeeKeystoresAliasesPkcs12_ApigeeKeystoresAliasesPkcs12Example|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample|TestAccAlloydbCluster_missingLocation|TestAccAlloydbBackup_missingLocation|TestAccComputeForwardingRule_update|TestAccDataSourceAlloydbLocations_basic|TestAccDataSourceGoogleFirebaseAndroidAppConfig |
|
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
| } | ||
| proximity { | ||
| window_before = 25 | ||
| windows_after = 25 |
There was a problem hiding this comment.
Changed to window_after.
I am not sure and going to ask my team. |
There was an issue with the missing test detector which this PR should fix. Here is the report after applying the fix: Resource: |
Thanks for the help, @trodge. I've added the remaining fields to the config. |
modular-magician
added
the
awaiting-approval
modular-magician
removed
the
awaiting-approval
|
/gcbrun |
modular-magician
added
awaiting-approval
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are. Terraform GA: Diff ( 4 files changed, 4794 insertions(+), 338 deletions(-)) |
Tests analyticsTotal tests: Action takenFound 8 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected testsTestAccFirebaserulesRelease_BasicRelease|TestAccComputeForwardingRule_update|TestAccAlloydbCluster_missingLocation|TestAccApigeeKeystoresAliasesKeyCertFile_apigeeKeystoresAliasesKeyCertFileTestExample|TestAccApigeeKeystoresAliasesPkcs12_ApigeeKeystoresAliasesPkcs12Example|TestAccAlloydbBackup_missingLocation|TestAccDataSourceAlloydbLocations_basic|TestAccDataSourceGoogleFirebaseAndroidAppConfig |
|
Tests passed during RECORDING mode: Tests failed during RECORDING mode: Please fix these to complete your PR |
* inspectConfig added to jobTrigger * Test Precheck Updated * fields added to test * removed deprecated attributes contentOptions and detectionRules * customInfoTypes tests added covering all attributes * tabs removed from erb file and double space added * removed required from mentioned attributes * minor changes * Additional test for Exclusion Rule added * removing required from hotwordRegex and pattern * Spacing changed * immutable removed from customInfoType attributes * required removed from proximity * required removed from proximity and likelihoodAdjustment in rules * Tests modified and new cases added * added remaining 4 attr to test config * minor changes
* inspectConfig added to jobTrigger * Test Precheck Updated * fields added to test * removed deprecated attributes contentOptions and detectionRules * customInfoTypes tests added covering all attributes * tabs removed from erb file and double space added * removed required from mentioned attributes * minor changes * Additional test for Exclusion Rule added * removing required from hotwordRegex and pattern * Spacing changed * immutable removed from customInfoType attributes * required removed from proximity * required removed from proximity and likelihoodAdjustment in rules * Tests modified and new cases added * added remaining 4 attr to test config * minor changes
* inspectConfig added to jobTrigger * Test Precheck Updated * fields added to test * removed deprecated attributes contentOptions and detectionRules * customInfoTypes tests added covering all attributes * tabs removed from erb file and double space added * removed required from mentioned attributes * minor changes * Additional test for Exclusion Rule added * removing required from hotwordRegex and pattern * Spacing changed * immutable removed from customInfoType attributes * required removed from proximity * required removed from proximity and likelihoodAdjustment in rules * Tests modified and new cases added * added remaining 4 attr to test config * minor changes
* inspectConfig added to jobTrigger * Test Precheck Updated * fields added to test * removed deprecated attributes contentOptions and detectionRules * customInfoTypes tests added covering all attributes * tabs removed from erb file and double space added * removed required from mentioned attributes * minor changes * Additional test for Exclusion Rule added * removing required from hotwordRegex and pattern * Spacing changed * immutable removed from customInfoType attributes * required removed from proximity * required removed from proximity and likelihoodAdjustment in rules * Tests modified and new cases added * added remaining 4 attr to test config * minor changes
* inspectConfig added to jobTrigger * Test Precheck Updated * fields added to test * removed deprecated attributes contentOptions and detectionRules * customInfoTypes tests added covering all attributes * tabs removed from erb file and double space added * removed required from mentioned attributes * minor changes * Additional test for Exclusion Rule added * removing required from hotwordRegex and pattern * Spacing changed * immutable removed from customInfoType attributes * required removed from proximity * required removed from proximity and likelihoodAdjustment in rules * Tests modified and new cases added * added remaining 4 attr to test config * minor changes
Added
inspectConfigblock with all the attributes toinspectJobinjobTriggerresource and also added relevant test cases.If this PR is for Terraform, I acknowledge that I have:
make testandmake lintin the generated providers to ensure it passes unit and linter tests.Release Note Template for Downstream PRs (will be copied)