tests: integrate pub/sub sample system tests that are still useful#8091
tests: integrate pub/sub sample system tests that are still useful#8091
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive testing framework for Pub/Sub, including unit tests for common utilities and system tests for core operations like topic creation and message publishing. It also adds a TestResources class to manage resource lifecycle and prevent naming conflicts. Key feedback includes addressing a potential memory leak in the message listener timeout logic, improving the testability of the cleanup utility by using the provided timestamp provider instead of a direct call to Date.now(), and removing redundant any type assertions for better type safety.
There was a problem hiding this comment.
The 'message' listener is not removed if the promise rejects due to a timeout. This can lead to memory leaks or unexpected behavior in the test suite if a message arrives after the timeout. It is better to explicitly remove the listener in the timeout path.
const message = await new Promise<Message>((resolve, reject) => {
const timeout = setTimeout(() => {
subscription.removeListener('message', messageHandler);
reject(new Error('Timeout'));
}, 10000);
function messageHandler(m: Message) {
clearTimeout(timeout);
m.ack();
resolve(m);
}
subscription.once('message', messageHandler);
});| if (name.startsWith(this.testSuiteId)) { | ||
| const parts = name.split('-'); | ||
| const createdAt = Number(parts[1]); | ||
| const timeDiff = (Date.now() - createdAt) / (1000 * 60 * 60); |
There was a problem hiding this comment.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

Work in progress for now.