Skip to content

Commit 64ec0c7

Browse files
hjmjohnsonhjmjohnson
committed
ENH: Refactor metadata handling in tests
Initial resolution for addressing ambiguitiy in 'char' signed behavior desicribed in issue Related to: #5779 `VerifyMetaData` function to eliminate redundant metadata handling code in `HDF5ImageIOTest` and `MetaDataDictionaryGTest`. This simplifies test logic and improves maintainability. Reduce duplication of common testing. Provide more failure diagnostics to assist with pin-pointing the failure locations. Replace `dynamic_cast` with `static_cast` and explicit type information checks for clearer and more efficient metadata handling. On linux-arm <char> and <unsigned char> are the same types. On most other platforms, <char> is the same as <signed char>.
1 parent 0ac9364 commit 64ec0c7

6 files changed

Lines changed: 470 additions & 158 deletions

File tree

‎Modules/Core/Common/include/itkMetaDataObject.hxx‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,132 @@ MetaDataObject<MetaDataObjectType>::Print(std::ostream & os) const
8383

8484
} // end namespace itk
8585

86+
namespace
87+
{
88+
/* A utilitiy function used for testing, this is not intended to be part of the public interface */
89+
/* Used only to avoid duplicate code in itkMetaDictionaryGTest.cxx and itkHDF5ImageIOTest.cxx */
90+
template <typename T>
91+
int
92+
VerifyMetaDataPrivateTestingUtility(const itk::MetaDataDictionary & metaDict,
93+
const std::string & key,
94+
const T & knownValue)
95+
{
96+
int status = EXIT_SUCCESS;
97+
T exposedValue{};
98+
99+
#if defined ITK_FUTURE_LEGACY_REMOVE
100+
static_assert(
101+
!std::is_same_v<itk::Array<char>, T>,
102+
"Should not use the ambiguous 'char' stored in meta data, because it is not-cross platform consistent.");
103+
static_assert(
104+
!std::is_same_v<char, T>,
105+
"Should not use the ambiguous 'char' stored in meta data, because it is not-cross platform consistent.");
106+
if (!itk::ExposeMetaData<T>(metaDict, key, exposedValue))
107+
{
108+
std::cerr << "Failure ExposeMetaData of boolean type '" << key << "'" << std::endl;
109+
status = EXIT_FAILURE;
110+
}
111+
#else
112+
if constexpr (std::is_same_v<itk::Array<char>, T>)
113+
{
114+
// If Encapsulate and Expose Metadata is all in core memory operations,
115+
// the type of char may be preserved.
116+
if (!itk::ExposeMetaData<itk::Array<char>>(metaDict, key, exposedValue))
117+
{
118+
// If Encapsulate and Expose Metadata is written to disk and
119+
// possibly shared across platforms, then 'char' may have been
120+
// stored in an intermediate format (aka file on disk) with explicit
121+
// signed or unsigned characteristics
122+
if constexpr (std::is_signed_v<char>)
123+
{
124+
itk::Array<signed char> temp_value{};
125+
if (!itk::ExposeMetaData<itk::Array<signed char>>(metaDict, key, temp_value))
126+
{
127+
std::cerr << "Failure ExposeMetaData '" << key << "'" << std::endl;
128+
status = EXIT_FAILURE;
129+
}
130+
exposedValue = temp_value;
131+
}
132+
else
133+
{
134+
itk::Array<unsigned char> temp_value{};
135+
if (!itk::ExposeMetaData<itk::Array<unsigned char>>(metaDict, key, temp_value))
136+
{
137+
std::cerr << "Failure ExposeMetaData '" << key << "'" << std::endl;
138+
status = EXIT_FAILURE;
139+
}
140+
exposedValue = temp_value;
141+
}
142+
}
143+
}
144+
else if constexpr (std::is_same_v<char, T>)
145+
{
146+
// If Encapsulate and Expose Metadata is all in core memory operations,
147+
// the type of char may be preserved.
148+
if (!itk::ExposeMetaData<char>(metaDict, key, exposedValue))
149+
{
150+
// If Encapsulate and Expose Metadata is written to disk and
151+
// possibly shared across platforms, then 'char' may have been
152+
// stored in an intermediate format (aka file on disk) with explicit
153+
// signed or unsigned characteristics
154+
if constexpr (std::is_signed_v<char>)
155+
{
156+
signed char temp_value{};
157+
158+
if (!itk::ExposeMetaData<signed char>(metaDict, key, temp_value))
159+
{
160+
std::cerr << "Failure ExposeMetaData '" << key << "'" << std::endl;
161+
status = EXIT_FAILURE;
162+
}
163+
exposedValue = static_cast<T>(temp_value);
164+
}
165+
else
166+
{
167+
unsigned char temp_value{};
168+
if (!itk::ExposeMetaData<unsigned char>(metaDict, key, temp_value))
169+
{
170+
std::cerr << "Failure ExposeMetaData '" << key << "'" << std::endl;
171+
status = EXIT_FAILURE;
172+
}
173+
exposedValue = static_cast<T>(temp_value);
174+
}
175+
}
176+
}
177+
else if (!itk::ExposeMetaData<T>(metaDict, key, exposedValue))
178+
{
179+
std::cerr << "Failure ExposeMetaData of boolean type '" << key << "'" << std::endl;
180+
status = EXIT_FAILURE;
181+
}
182+
#endif
183+
184+
185+
if constexpr (std::is_floating_point_v<T>)
186+
{
187+
if (itk::Math::NotAlmostEquals(exposedValue, knownValue))
188+
{
189+
std::cerr << "Incorrect meta value read in for " << key << " '" << exposedValue << "' != '" << knownValue << "'"
190+
<< std::endl;
191+
status = EXIT_FAILURE;
192+
}
193+
}
194+
else
195+
{
196+
if (exposedValue != knownValue)
197+
{
198+
std::cerr << "Incorrect meta value read in for " << key << " '" << exposedValue << "' != '" << knownValue << "'"
199+
<< std::endl;
200+
status = EXIT_FAILURE;
201+
}
202+
}
203+
if (status == EXIT_FAILURE)
204+
{
205+
std::cerr << "========================================" << std::endl;
206+
metaDict.Print(std::cerr);
207+
std::cerr << "========================================" << std::endl;
208+
}
209+
return status;
210+
}
211+
} // namespace
212+
213+
86214
#endif

‎Modules/Core/Common/test/itkMetaDataDictionaryGTest.cxx‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,112 @@ createMetaDataDictionary()
4444
return metaDataDictionary;
4545
}
4646

47+
48+
template <typename T>
49+
static int
50+
CheckMetaData(itk::MetaDataDictionary & metaDict, const std::string & key, const T & knownValue)
51+
{
52+
itk::EncapsulateMetaData<T>(metaDict, key, knownValue);
53+
return VerifyMetaDataPrivateTestingUtility<T>(metaDict, key, knownValue);
54+
}
55+
56+
57+
static int
58+
doExposeMetaDatas()
59+
{
60+
// Simplified version of tests found in HDF5 reading/writing
61+
// that are broken out here to improve localization of bugs
62+
// found during linux-arm building
63+
itk::MetaDataDictionary metaDict;
64+
int success = EXIT_SUCCESS;
65+
66+
if (CheckMetaData<bool>(metaDict, "TestBool", false) != EXIT_SUCCESS)
67+
{
68+
success = EXIT_FAILURE;
69+
}
70+
#if !defined(ITK_FUTURE_LEGACY_REMOVE)
71+
if (CheckMetaData<char>(metaDict, "TestChar", 'c') != EXIT_SUCCESS)
72+
{
73+
success = EXIT_FAILURE;
74+
}
75+
#endif
76+
if (CheckMetaData<unsigned char>(metaDict, "TestUChar", 'u') != EXIT_SUCCESS)
77+
{
78+
success = EXIT_FAILURE;
79+
}
80+
if (CheckMetaData<short>(metaDict, "TestShort", 1) != EXIT_SUCCESS)
81+
{
82+
success = EXIT_FAILURE;
83+
}
84+
if (CheckMetaData<unsigned short>(metaDict, "TestUShort", 3) != EXIT_SUCCESS)
85+
{
86+
success = EXIT_FAILURE;
87+
}
88+
if (CheckMetaData<int>(metaDict, "TestInt", 5) != EXIT_SUCCESS)
89+
{
90+
success = EXIT_FAILURE;
91+
}
92+
if (CheckMetaData<unsigned int>(metaDict, "TestUInt", 7) != EXIT_SUCCESS)
93+
{
94+
success = EXIT_FAILURE;
95+
}
96+
if (CheckMetaData<long>(metaDict, "TestLong", 5) != EXIT_SUCCESS)
97+
{
98+
success = EXIT_FAILURE;
99+
}
100+
if (CheckMetaData<unsigned long>(metaDict, "TestULong", 7) != EXIT_SUCCESS)
101+
{
102+
success = EXIT_FAILURE;
103+
}
104+
if (CheckMetaData<long long>(metaDict, "TestLLong", -5) != EXIT_SUCCESS)
105+
{
106+
success = EXIT_FAILURE;
107+
}
108+
if (CheckMetaData<unsigned long long>(metaDict, "TestULLong", 7ull) != EXIT_SUCCESS)
109+
{
110+
success = EXIT_FAILURE;
111+
}
112+
if (CheckMetaData<float>(metaDict, "TestFloat", 1.23456f) != EXIT_SUCCESS)
113+
{
114+
success = EXIT_FAILURE;
115+
}
116+
if (CheckMetaData<double>(metaDict, "TestDouble", 1.23456) != EXIT_SUCCESS)
117+
{
118+
success = EXIT_FAILURE;
119+
}
120+
121+
#if !defined(ITK_FUTURE_LEGACY_REMOVE)
122+
itk::Array<char> metaDataCharArray(5);
123+
metaDataCharArray[0] = 'h';
124+
metaDataCharArray[1] = 'e';
125+
metaDataCharArray[2] = 'l';
126+
metaDataCharArray[3] = 'l';
127+
metaDataCharArray[4] = 'o';
128+
if (CheckMetaData<itk::Array<char>>(metaDict, "TestCharArray", metaDataCharArray) != EXIT_SUCCESS)
129+
{
130+
success = EXIT_FAILURE;
131+
}
132+
#endif
133+
134+
itk::Array<double> metaDataDoubleArray(5);
135+
metaDataDoubleArray[0] = 3.0;
136+
metaDataDoubleArray[1] = 1.0;
137+
metaDataDoubleArray[2] = 4.0;
138+
metaDataDoubleArray[3] = 5.0;
139+
metaDataDoubleArray[4] = 2.0;
140+
if (CheckMetaData<itk::Array<double>>(metaDict, "TestDoubleArray", metaDataDoubleArray) != EXIT_SUCCESS)
141+
{
142+
success = EXIT_FAILURE;
143+
}
144+
145+
if (CheckMetaData<std::string>(metaDict, "StdString", "Test std::string") != EXIT_SUCCESS)
146+
{
147+
success = EXIT_FAILURE;
148+
}
149+
return success;
150+
}
151+
152+
47153
template <typename T>
48154
itk::MetaDataObjectBase::Pointer
49155
createMetaDataObject(const T & invalue)
@@ -56,6 +162,9 @@ createMetaDataObject(const T & invalue)
56162

57163
TEST(MetaDataDictionary, Basic)
58164
{
165+
// Isolate
166+
EXPECT_EQ(doExposeMetaDatas(), EXIT_SUCCESS);
167+
59168
// This test exercises and checks the non-constant interface
60169
itk::MetaDataDictionary dic = createMetaDataDictionary();
61170

0 commit comments

Comments
 (0)