Skip to content

Fix CL tests#7074

Open
psamanoelton wants to merge 2 commits intotensorflow:masterfrom
psamanoelton:fix_cl
Open

Fix CL tests#7074
psamanoelton wants to merge 2 commits intotensorflow:masterfrom
psamanoelton:fix_cl

Conversation

@psamanoelton
Copy link

Summary

Fix failing TensorBoard tests caused by compatibility issues with newer dependency versions.

Two issues were identified when running the TensorBoard test suite:

  • Moto API change

    • Recent versions of moto no longer expose mock_s3, replacing it with the unified mock_aws decorator. The S3 test was updated accordingly.
  • assertProtoEquals incompatibility with newer protobuf runtime

    • Some tests used TensorFlow’s assertProtoEquals, which internally relies on descriptor fields that are not available in the current protobuf/upb runtime. This caused tests to fail with:
      AttributeError: 'google._upb._message.FieldDescriptor' object has no attribute 'label'

The affected tests were updated to parse the expected text-format protobuf and compare the resulting proto objects directly, avoiding the incompatible helper.

Additionally, the expected float value in the tests was adjusted to match the current floating-point representation produced during aggregation.

response,
expected,
)
self.assertEqual(response, expected)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this error is basically making evident that the protos used are not compatible with the environment where they're used.

Changing the way this validation is done just works around the incompatibility that uses could otherwise encounter, although to be honest, I'm not sure how likely this is to be an issue.

I would prefer to try to make the assertProtoEquals work (I think this will result in the challenging dependency update that I encountered before).

If we can't get it to work, we can consider adding this workaround for now, but adding a short comment clarifying why the asserProtosEqual is not being used.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion — I also tried to see if we could keep using assertProtoEquals.

The issue appears to come from the protobuf runtime installed in the current test environment. CI installs tf-nightly, which pulls in protobuf 7.x. In that version the FieldDescriptor.label attribute is no longer available:

Has FieldDescriptor.label: False

However, TensorFlow’s assertProtoEquals helper still relies on that attribute internally, which causes it to fail before actually comparing the protos:

AttributeError: 'google._upb._message.FieldDescriptor' object has no attribute 'label'

Since tf-nightly requires a recent protobuf version, fixing this by pinning protobuf to an older version would likely require broader dependency or workflow changes.

I also added a short comment in the test explaining why assertProtoEquals is avoided here.

If you prefer, we could also explore restoring assertProtoEquals through a dependency adjustment or compatibility patch in a follow-up change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants