Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

gloo_test test_close_connection not working as intended due to unwanted comma #70609

Open
code-review-doctor opened this issue Jan 4, 2022 · 0 comments
Labels
caffe2 triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@code-review-doctor
Copy link
Contributor

馃悰 Describe the bug

There is an unwanted comma on this line

closer = comm_rank == comm_size // 2,

This makes closer always evaluate to a tuple containing one item: either (True,) or (False,). Notice therefore that closer will always evaluate to truthy because a non-empty tuple is always truthy.

This is a problem because the following code will never be hit because closer is never falsey:

if not closer:
net.Barrier(
[common_world],
[],
engine=op_engine)

so it looks like only closing path is covered in this test.

The solution is to remove the unwanted comma. I can make a PR for this?

Versions

NA

@VitalyFedyunin VitalyFedyunin added caffe2 triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
caffe2 triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

No branches or pull requests

2 participants