-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add tests to Cloud Tasks Samples #1459
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
Conversation
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'm presuming this is because the wrong copyright date was uploaded. We don't change copyrights if they were correct (ie. we keep the earliest version)
|
||
// Instantiates a client. | ||
try (CloudTasksClient client = CloudTasksClient.create()) { | ||
// Variables provided by the system variables. | ||
// projectId = "my-project-id"; | ||
// queueName = "my-queue"; | ||
// location = "us-central1"; | ||
// url = "https://example.com/taskhandler"; | ||
String url = "https://example.com/taskhandler"; |
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.
Do you call this out in the README? (CHANGEME)
// url = "https://example.com/taskhandler"; | ||
String payload = "hello"; | ||
String email = args[0]; // Cloud IAM service account | ||
String url = |
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.
Should we suggest changing this?
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.
Since this is a snippet. I have updated with the sample without a main and highlighted the parameters.
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.
A couple of nits
|
||
private ByteArrayOutputStream bout; | ||
private PrintStream out; | ||
// @Rule public Timeout globalTimeout = Timeout.seconds(300); // 5 minute timeout |
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.
Kept intentionally?
@lesv please review changes |
* Add tests * Update license * Update README * Update for testing purposes * Update queue location * Fix test styling * Library version * Update service account * Fix service account
* Add tests * Update license * Update README * Update for testing purposes * Update queue location * Fix test styling * Library version * Update service account * Fix service account
No description provided.