Skip to content

Fix CL tests #7074

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.

Sign up for GitHub

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

Open
psamanoelton wants to merge 2 commits into
base: master
Choose a base branch
from
+93 −62
Open

Fix CL tests #7074

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f1323a6
Update boto and fix tests
psamanoelton Mar 10, 2026
d11d1d9
Solving issue with pip and adding comment for avoid assertProtoEquals…
psamanoelton Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ jobs:
fail-fast: false
matrix:
tf_version_id: ['tf', 'notf']
python_version: ['3.9']
python_version: ['3.10']
steps:
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0
- uses: actions/setup-python@13ae5bb136fac2878aff31522b9efb785519f984 # v4.3.0
Expand Down Expand Up @@ -148,7 +148,7 @@ jobs:
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0
- uses: actions/setup-python@13ae5bb136fac2878aff31522b9efb785519f984 # v4.3.0
with:
python-version: '3.9'
python-version: '3.10'
architecture: 'x64'
- name: 'Cache Cargo artifacts'
if: matrix.mode == 'native'
Expand Down Expand Up @@ -216,7 +216,7 @@ jobs:
# flake8 should run on each Python version that we target,
# because the errors and warnings can differ due to language
# changes, and we want to catch them all.
python_version: ['3.9', '3.10', '3.11']
python_version: ['3.10', '3.11']
steps:
- uses: actions/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c # v3.3.0
- uses: actions/setup-python@13ae5bb136fac2878aff31522b9efb785519f984 # v4.3.0
Expand Down
36 changes: 18 additions & 18 deletions tensorboard/compat/tensorflow_stub/io/gfile_s3_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import boto3
import os
import unittest
from moto import mock_s3
from moto import mock_aws

from tensorboard.compat.tensorflow_stub import errors
from tensorboard.compat.tensorflow_stub.io import gfile
Expand All @@ -29,14 +29,14 @@


class GFileTest(unittest.TestCase):
@mock_s3
@mock_aws
def testExists(self):
temp_dir = self._CreateDeepS3Structure()
ckpt_path = self._PathJoin(temp_dir, "model.ckpt")
self.assertTrue(gfile.exists(temp_dir))
self.assertTrue(gfile.exists(ckpt_path))

@mock_s3
@mock_aws
def testGlob(self):
temp_dir = self._CreateDeepS3Structure()
# S3 glob includes subdirectory content, which standard
Expand Down Expand Up @@ -65,12 +65,12 @@ def testGlob(self):
% (expected_listing, gotten_listing),
)

@mock_s3
@mock_aws
def testIsdir(self):
temp_dir = self._CreateDeepS3Structure()
self.assertTrue(gfile.isdir(temp_dir))

@mock_s3
@mock_aws
def testListdir(self):
temp_dir = self._CreateDeepS3Structure()
self._CreateDeepS3Structure(temp_dir)
Expand All @@ -86,20 +86,20 @@ def testListdir(self):
gotten_files = gfile.listdir(temp_dir)
self.assertCountEqual(expected_files, gotten_files)

@mock_s3
@mock_aws
def testMakeDirs(self):
temp_dir = self._CreateDeepS3Structure()
new_dir = self._PathJoin(temp_dir, "newdir", "subdir", "subsubdir")
gfile.makedirs(new_dir)
self.assertTrue(gfile.isdir(new_dir))

@mock_s3
@mock_aws
def testMakeDirsAlreadyExists(self):
temp_dir = self._CreateDeepS3Structure()
new_dir = self._PathJoin(temp_dir, "bar", "baz")
gfile.makedirs(new_dir)

@mock_s3
@mock_aws
def testWalk(self):
temp_dir = self._CreateDeepS3Structure()
self._CreateDeepS3Structure(temp_dir)
Expand Down Expand Up @@ -173,7 +173,7 @@ def testWalk(self):
gotten = gfile.walk(temp_dir)
self._CompareFilesPerSubdirectory(expected, gotten)

@mock_s3
@mock_aws
def testStat(self):
ckpt_content = "asdfasdfasdffoobarbuzz"
temp_dir = self._CreateDeepS3Structure(ckpt_content=ckpt_content)
Expand All @@ -184,7 +184,7 @@ def testStat(self):
with self.assertRaises(errors.NotFoundError):
gfile.stat(bad_ckpt_path)

@mock_s3
@mock_aws
def testRead(self):
ckpt_content = "asdfasdfasdffoobarbuzz"
temp_dir = self._CreateDeepS3Structure(ckpt_content=ckpt_content)
Expand All @@ -194,7 +194,7 @@ def testRead(self):
ckpt_read = f.read()
self.assertEqual(ckpt_content, ckpt_read)

@mock_s3
@mock_aws
def testReadLines(self):
ckpt_lines = ["\n"] + ["line {}\n".format(i) for i in range(10)] + [" "]
ckpt_content = "".join(ckpt_lines)
Expand All @@ -205,7 +205,7 @@ def testReadLines(self):
ckpt_read_lines = list(f)
self.assertEqual(ckpt_lines, ckpt_read_lines)

@mock_s3
@mock_aws
def testReadWithOffset(self):
ckpt_content = "asdfasdfasdffoobarbuzz"
ckpt_b_content = b"asdfasdfasdffoobarbuzz"
Expand All @@ -227,7 +227,7 @@ def testReadWithOffset(self):
ckpt_read = f.read()
self.assertEqual(ckpt_b_content, ckpt_read)

@mock_s3
@mock_aws
def testWrite(self):
temp_dir = self._CreateDeepS3Structure()
ckpt_path = os.path.join(temp_dir, "model2.ckpt")
Expand All @@ -238,7 +238,7 @@ def testWrite(self):
ckpt_read = f.read()
self.assertEqual(ckpt_content, ckpt_read)

@mock_s3
@mock_aws
def testOverwrite(self):
temp_dir = self._CreateDeepS3Structure()
ckpt_path = os.path.join(temp_dir, "model2.ckpt")
Expand All @@ -251,7 +251,7 @@ def testOverwrite(self):
ckpt_read = f.read()
self.assertEqual(ckpt_content, ckpt_read)

@mock_s3
@mock_aws
def testWriteMultiple(self):
temp_dir = self._CreateDeepS3Structure()
ckpt_path = os.path.join(temp_dir, "model2.ckpt")
Expand All @@ -266,7 +266,7 @@ def testWriteMultiple(self):
ckpt_read = f.read()
self.assertEqual(ckpt_content, ckpt_read)

@mock_s3
@mock_aws
def testWriteEmpty(self):
temp_dir = self._CreateDeepS3Structure()
ckpt_path = os.path.join(temp_dir, "model2.ckpt")
Expand All @@ -277,7 +277,7 @@ def testWriteEmpty(self):
ckpt_read = f.read()
self.assertEqual(ckpt_content, ckpt_read)

@mock_s3
@mock_aws
def testWriteBinary(self):
temp_dir = self._CreateDeepS3Structure()
ckpt_path = os.path.join(temp_dir, "model.ckpt")
Expand All @@ -288,7 +288,7 @@ def testWriteBinary(self):
ckpt_read = f.read()
self.assertEqual(ckpt_content, ckpt_read)

@mock_s3
@mock_aws
def testWriteMultipleBinary(self):
temp_dir = self._CreateDeepS3Structure()
ckpt_path = os.path.join(temp_dir, "model2.ckpt")
Expand Down
5 changes: 3 additions & 2 deletions tensorboard/pip_package/requirements_dev.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
grpcio-testing==1.24.3
pandas~=2.0
# For gfile S3 test
boto3==1.9.86
moto==1.3.7
boto3>=1.34
moto>=5

# For gfile fsspec test
fsspec>=2021.06.0

Expand Down
102 changes: 66 additions & 36 deletions tensorboard/plugins/hparams/list_session_groups_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,9 @@ def test_no_filter_no_sort(self):
aggregation_type: AGGREGATION_AVG
"""
response = self._run_handler(request)
self.assertProtoEquals(

expected = type(response)()
text_format.Parse(
"""
session_groups {
name: "group_1"
Expand All @@ -391,11 +393,15 @@ def test_no_filter_no_sort(self):
training_step: 1
wall_time_secs: 1.0
}
metric_values { name { tag: "delta_temp" } value: 15
metric_values {
name { tag: "delta_temp" }
value: 15
training_step: 2
wall_time_secs: 10.0
}
metric_values { name { tag: "optional_metric" } value: 33
metric_values {
name { tag: "optional_metric" }
value: 33
training_step: 20
wall_time_secs: 2.0
}
Expand All @@ -416,7 +422,6 @@ def test_no_filter_no_sort(self):
training_step: 2
wall_time_secs: 10.0
}

metric_values {
name { tag: "optional_metric" }
value: 33
Expand All @@ -441,7 +446,7 @@ def test_no_filter_no_sort(self):
name { tag: "delta_temp" }
value: 44.5
training_step: 2
wall_time_secs: 10.3333333
wall_time_secs: 10.333333333333334
}
sessions {
name: "session_2"
Expand All @@ -454,7 +459,8 @@ def test_no_filter_no_sort(self):
training_step: 1
wall_time_secs: 1.0
}
metric_values { name { tag: "delta_temp" }
metric_values {
name { tag: "delta_temp" }
value: 150
training_step: 3
wall_time_secs: 11.0
Expand All @@ -471,7 +477,8 @@ def test_no_filter_no_sort(self):
training_step: 1
wall_time_secs: 1.0
}
metric_values { name { tag: "delta_temp" }
metric_values {
name { tag: "delta_temp" }
value: 1.5
training_step: 2
wall_time_secs: 10.0
Expand All @@ -488,7 +495,8 @@ def test_no_filter_no_sort(self):
training_step: 1
wall_time_secs: 1.0
}
metric_values { name { tag: "delta_temp" }
metric_values {
name { tag: "delta_temp" }
value: -18
training_step: 2
wall_time_secs: 10.0
Expand All @@ -502,15 +510,18 @@ def test_no_filter_no_sort(self):
hparams { key: "initial_temp" value { number_value: 300.0 } }
hparams { key: "string_hparam" value { string_value: "a string_3"}}
hparams {
key: 'optional_string_hparam' value { string_value: 'BB' }
key: "optional_string_hparam"
value { string_value: "BB" }
}
metric_values {
name { tag: "current_temp" }
value: 101.0
training_step: 1
wall_time_secs: 1.0
}
metric_values { name { tag: "delta_temp" } value: -151.0
metric_values {
name { tag: "delta_temp" }
value: -151.0
training_step: 2
wall_time_secs: 10.0
}
Expand All @@ -525,16 +536,21 @@ def test_no_filter_no_sort(self):
training_step: 1
wall_time_secs: 1.0
}
metric_values { name { tag: "delta_temp" } value: -151.0
metric_values {
name { tag: "delta_temp" }
value: -151.0
training_step: 2
wall_time_secs: 10.0
}
}
}
total_size: 3
""",
response,
expected,
)
# Avoid assertProtoEquals because TensorFlow's helper is incompatible with
# protobuf >= 7 (FieldDescriptor.label removed in upb runtime).
self.assertEqual(response, expected)
Copy link
Member

@arcra arcra Mar 10, 2026

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

@psamanoelton psamanoelton Mar 10, 2026

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.


def test_no_allowed_statuses(self):
request = """
Expand Down Expand Up @@ -2179,39 +2195,53 @@ def test_experiment_from_data_provider_with_metric_values_aggregates(
"""
response = self._run_handler(request)
self.assertLen(response.session_groups[0].metric_values, 3)
self.assertProtoEquals(

actual = response.session_groups[0].metric_values[0]
expected = type(actual)()
text_format.Parse(
"""
name {
tag: "current_temp"
}
value: 37.0
training_step: 1
wall_time_secs: 1.0
name {
tag: "current_temp"
}
value: 37.0
training_step: 1
wall_time_secs: 1.0
""",
response.session_groups[0].metric_values[0],
expected,
)
self.assertProtoEquals(
self.assertEqual(actual, expected)

actual = response.session_groups[0].metric_values[1]
expected = type(actual)()
text_format.Parse(
"""
name {
tag: "delta_temp"
}
value: 55.5
training_step: 2
wall_time_secs: 10.3333333
name {
tag: "delta_temp"
}
value: 55.5
training_step: 2
wall_time_secs: 10.333333333333334
""",
response.session_groups[0].metric_values[1],
expected,
)
self.assertProtoEquals(
self.assertEqual(actual, expected)

actual = response.session_groups[0].metric_values[2]
expected = type(actual)()
text_format.Parse(
"""
name {
tag: "optional_metric"
}
value: 33.0
training_step: 20
wall_time_secs: 2.0
name {
tag: "optional_metric"
}
value: 33.0
training_step: 20
wall_time_secs: 2.0
""",
response.session_groups[0].metric_values[2],
expected,
)
# Avoid assertProtoEquals because TensorFlow's helper is incompatible with
# protobuf >= 7 (FieldDescriptor.label removed in upb runtime).
self.assertEqual(actual, expected)

def test_experiment_from_data_provider_filters_by_metric_values(
self,
Expand Down
Loading
Loading