-
-
Notifications
You must be signed in to change notification settings - Fork 27
Add publish selector test and remove debug output #367
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
base: publishWithSelector
Are you sure you want to change the base?
Add publish selector test and remove debug output #367
Conversation
|
@codex review logic of publishWithSelector. |
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.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| assertEquals( | ||
| listOf(Pair(1, "odd"), Pair(2, "even"), Pair("4", "string")), | ||
| result, |
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.
[P1] Expected list ignores shared selector emissions
This test merges four selectors, but the first selector (interval(0L, 100L)…withLatestFrom(sharedFlow).takeUntil(sharedFlow.filter { it == 3 })) emits at least (1, "shared") and (2, "shared") before the shared flow reaches 3. Because the assertion only includes the odd, even, and string pairs, the collected result will contain additional elements and the test will fail whenever it is run. Either include the shared selector’s outputs in the expected list or modify that selector so it deliberately produces no values.
Useful? React with 👍 / 👎.
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.
Pull Request Overview
This PR cleans up the publishWithSelector implementation by removing debug output and an embedded sample main function, while adding proper test coverage for the publish selector functionality.
- Remove temporary debugging println statements and sample main function from the publishWithSelector implementation
- Add comprehensive test coverage for publish with multiple selector flows
- Update SimpleLazy class documentation and public API dump
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| PublishWithSelectorTest.kt | Adds new test file with regression test covering publish with multiple selector flows |
| publishWithSelector.kt | Removes debug println statements, unused imports, and embedded sample main function |
| SimpleLazy.kt | Updates comment to clarify the use of SynchronizedObject for multiplatform synchronization |
| FlowExt.api | Removes main function entries from public API dump |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Summary
mainfrompublishWithSelectorSimpleLazysynchronization comment and update the public API dumppublishwith multiple selector flowsTesting
https://chatgpt.com/codex/tasks/task_e_68c81f06db64832e8b42acce81a672df