pubsub: fix testStartStopReset #1453
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The test verifies that calling stop() actually stops the emulator. The previous implementation stops the emulator by - sending HTTP POST to /shutdown - terminating the emulator process The second step follows directly after the first. It then queries the server it just shut down to make sure it does not respond. This behavior is incorrect for two reasons. - The server might continue serving request after responding to /shutdown. - The "emulator process" might not be the emulator at all. It might be a script that in-turn runs the emulator. On UNIX systems, killing a parent process does not kill any of its children; the actual emulator might run indefinitely after we kill the process we spawned. This commit changes this behavior. After POSTing to /shutdown, we simply wait for the spawned process to terminate on its own. Technically, there is still a race: Another emulator could have been started on the same port. When we send a request to verify that the old emulator has terminated, we might get a response from the new one. Since we execute tests sequentially though, I believe this should not be a problem in practice. Updates #1429.
googlebot
added
the
cla: yes
| if (process != null) { | ||
| process.destroy(); | ||
| process.waitFor(); |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
|
@garrettjonesgoogle PTAL, we now raises an exception if process has not finished in specified time. |
raises exception if we cannot stop the emulator.
|
Changes Unknown when pulling 84b12ba on pongad:pubsub-test into ** on GoogleCloudPlatform:master**. |
github-actions bot
pushed a commit
that referenced
this pull request
Aug 25, 2022
🤖 I have created a release *beep* *boop* --- ## [2.3.6](googleapis/java-bigquerydatatransfer@v2.3.5...v2.3.6) (2022-08-24) ### Dependencies * update dependency com.google.cloud:google-cloud-pubsub to v1.120.13 ([#1452](googleapis/java-bigquerydatatransfer#1452)) ([29121bf](googleapis/java-bigquerydatatransfer@29121bf)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The test verifies that calling stop() actually stops the emulator.
The previous implementation stops the emulator by
The second step follows directly after the first.
It then queries the server it just shut down to make sure it does not
respond.
This behavior is incorrect for two reasons.
/shutdown.
It might be a script that in-turn runs the emulator.
On UNIX systems, killing a parent process does not kill any of its
children; the actual emulator might run indefinitely after
we kill the process we spawned.
This commit changes this behavior.
After POSTing to /shutdown, we simply wait for the spawned process
to terminate on its own.
Technically, there is still a race:
Another emulator could have been started on the same port.
When we send a request to verify that the old emulator has terminated,
we might get a response from the new one.
Since we execute tests sequentially though,
I believe this should not be a problem in practice.
Updates #1429.