-
Notifications
You must be signed in to change notification settings - Fork 0
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: add bigquery export samples #7
Conversation
|
||
package vtwo.bigquery; | ||
|
||
// [START securitycenter_create_bigquery_export_v2] |
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.
This tag should enclose only the function public static void createBigQueryExport(
, no?
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.
it makes sense if we follow the same example we used in GetIamPolicies:
https://github.com/GoogleCloudPlatform/java-docs-samples/blob/main/security-command-center/snippets/src/main/java/vtwo/iam/GetIamPolicies.java
but I'm worried about the change, as the same PR is already approved in the same syntax.
GoogleCloudPlatform#9291
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.
ok, no problem
SecurityCenterClient client = mock(SecurityCenterClient.class); | ||
try (MockedStatic<SecurityCenterClient> clientMock = Mockito.mockStatic( | ||
SecurityCenterClient.class)) { | ||
clientMock.when(SecurityCenterClient::create).thenReturn(client); |
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 guess these lines could be put in a @before method, once it's used in all of the tests.
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.
done
try (MockedStatic<SecurityCenterClient> clientMock = Mockito.mockStatic( | ||
SecurityCenterClient.class)) { | ||
clientMock.when(SecurityCenterClient::create).thenReturn(client); | ||
|
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.
delete extra line here
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.
done
|
||
// Define test data. | ||
String filter = "test-filter"; | ||
|
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.
delete extra line
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.
done
// Build the parent of the request. | ||
OrganizationLocationName organizationName = OrganizationLocationName.of(ORGANIZATION_ID, | ||
LOCATION); | ||
|
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.
delete extra line
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.
done
.setFilter(filter) | ||
.setDataset(String.format("projects/%s/datasets/%s", PROJECT_ID, BQ_DATASET_NAME)) | ||
.build(); | ||
|
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.
delete extra line
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.
done
.setBigQueryExport(expectedExport) | ||
.setBigQueryExportId(BQ_EXPORT_ID) | ||
.build(); | ||
|
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.
delete extra line
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.
done
} | ||
|
||
@Test | ||
public void testDeleteBigQueryExport() throws IOException { |
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.
please do the same I asked for the first test on the tests below
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.
done
BQ_DATASET_NAME, BQ_EXPORT_ID); | ||
|
||
// Verify that the createBigQueryExport method was called with the expected request. | ||
Mockito.verify(client).createBigQueryExport(expectedRequest); |
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.
Please statically import Mockito.verify()
so you can write this line as verify(client).createBigQueryExport(expectedRequest);
Same below!
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.
done
.build(); | ||
|
||
// Mock the createBigQueryExport method to return the expected response. | ||
Mockito.when(client.createBigQueryExport(any())).thenReturn(expectedExport); |
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.
Please statically import Mockito.when()
so you can write this line as when(client.createBigQueryExport(any())).thenReturn(expectedExport);
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.
done
|
||
package vtwo.bigquery; | ||
|
||
// [START securitycenter_create_bigquery_export_v2] |
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.
ok, no problem
.build(); | ||
// Mock the deleteBigQueryExport method to return successfully. | ||
doNothing().when(client).deleteBigQueryExport(bigQueryExportRequest); | ||
// Call the deleteBigQueryExport method. |
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.
Add line above, remember http://go/unit-testing-practices#structure
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.
done.
doNothing().when(client).deleteBigQueryExport(bigQueryExportRequest); | ||
// Call the deleteBigQueryExport method. | ||
DeleteBigQueryExport.deleteBigQueryExport(ORGANIZATION_ID, LOCATION, BQ_EXPORT_ID); | ||
// Verify that the deleteBigQueryExport method was called with the expected request. |
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.
Add line above, remember http://go/unit-testing-practices#structure
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.
done.
SecurityCenterClient client = mock(SecurityCenterClient.class); | ||
clientMock.when(SecurityCenterClient::create).thenReturn(client); |
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.
Can you also move these two lines to the setUp() method? (same for other methods)
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 found a way to move the two lines. The other attempts didn't work.
55085b4
to
8e9196e
Compare
8e9196e
to
280f187
Compare
…eleteBigQueryExport.java Co-authored-by: Sita Lakshmi Sangameswaran <sitalakshmi@google.com>
…etBigQueryExport.java Co-authored-by: Sita Lakshmi Sangameswaran <sitalakshmi@google.com>
…istBigQueryExports.java Co-authored-by: Sita Lakshmi Sangameswaran <sitalakshmi@google.com>
…pdateBigQueryExport.java Co-authored-by: Sita Lakshmi Sangameswaran <sitalakshmi@google.com>
9faabd1
to
433599c
Compare
Description
Fixes #
Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.
Checklist
pom.xml
parent set to latestshared-configuration
mvn clean verify
requiredmvn -P lint checkstyle:check
requiredmvn -P lint clean compile pmd:cpd-check spotbugs:check
advisory only