Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive test coverage for the tensor layout interface, validating layout functionality across multiple namespaces (c10, at, torch) and testing layout enum properties and output stream operations.
Key Changes:
- Added
<sstream>include for output stream testing - Implemented 6 new test cases covering layout constants, enum values, and stream operators
- Tests verify layout consistency across c10, at, and torch namespaces
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
test/TensorTest.cpp
Outdated
|
|
||
| oss.str(""); | ||
| oss << c10::Layout::Strided; | ||
| EXPECT_EQ(oss.str(), "Strided"); | ||
|
|
||
| oss.str(""); | ||
| oss << c10::Layout::Sparse; | ||
| EXPECT_EQ(oss.str(), "Sparse"); | ||
|
|
||
| oss.str(""); | ||
| oss << c10::Layout::SparseCsr; | ||
| EXPECT_EQ(oss.str(), "SparseCsr"); | ||
|
|
||
| oss.str(""); | ||
| oss << c10::Layout::SparseCsc; | ||
| EXPECT_EQ(oss.str(), "SparseCsc"); | ||
|
|
||
| oss.str(""); | ||
| oss << c10::Layout::SparseBsr; | ||
| EXPECT_EQ(oss.str(), "SparseBsr"); | ||
|
|
||
| oss.str(""); | ||
| oss << c10::Layout::SparseBsc; | ||
| EXPECT_EQ(oss.str(), "SparseBsc"); | ||
|
|
||
| oss.str(""); | ||
| oss << c10::Layout::Mkldnn; | ||
| EXPECT_EQ(oss.str(), "Mkldnn"); | ||
|
|
||
| oss.str(""); | ||
| oss << c10::Layout::Jagged; | ||
| EXPECT_EQ(oss.str(), "Jagged"); |
There was a problem hiding this comment.
The test LayoutOutputStream contains repetitive code that clears and writes to the same ostringstream. This reduces maintainability. Consider using a helper function or restructuring the test to reduce duplication.
| oss.str(""); | |
| oss << c10::Layout::Strided; | |
| EXPECT_EQ(oss.str(), "Strided"); | |
| oss.str(""); | |
| oss << c10::Layout::Sparse; | |
| EXPECT_EQ(oss.str(), "Sparse"); | |
| oss.str(""); | |
| oss << c10::Layout::SparseCsr; | |
| EXPECT_EQ(oss.str(), "SparseCsr"); | |
| oss.str(""); | |
| oss << c10::Layout::SparseCsc; | |
| EXPECT_EQ(oss.str(), "SparseCsc"); | |
| oss.str(""); | |
| oss << c10::Layout::SparseBsr; | |
| EXPECT_EQ(oss.str(), "SparseBsr"); | |
| oss.str(""); | |
| oss << c10::Layout::SparseBsc; | |
| EXPECT_EQ(oss.str(), "SparseBsc"); | |
| oss.str(""); | |
| oss << c10::Layout::Mkldnn; | |
| EXPECT_EQ(oss.str(), "Mkldnn"); | |
| oss.str(""); | |
| oss << c10::Layout::Jagged; | |
| EXPECT_EQ(oss.str(), "Jagged"); | |
| auto checkLayoutToString = [&oss](c10::Layout layout, const std::string& expected) { | |
| oss.str(""); | |
| oss.clear(); | |
| oss << layout; | |
| EXPECT_EQ(oss.str(), expected); | |
| }; | |
| checkLayoutToString(c10::Layout::Strided, "Strided"); | |
| checkLayoutToString(c10::Layout::Sparse, "Sparse"); | |
| checkLayoutToString(c10::Layout::SparseCsr, "SparseCsr"); | |
| checkLayoutToString(c10::Layout::SparseCsc, "SparseCsc"); | |
| checkLayoutToString(c10::Layout::SparseBsr, "SparseBsr"); | |
| checkLayoutToString(c10::Layout::SparseBsc, "SparseBsc"); | |
| checkLayoutToString(c10::Layout::Mkldnn, "Mkldnn"); | |
| checkLayoutToString(c10::Layout::Jagged, "Jagged"); |
test/TensorTest.cpp
Outdated
| // 测试 layout 枚举值 | ||
| TEST_F(TensorTest, LayoutEnumValues) { | ||
| // 测试 Layout 枚举的底层值 | ||
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::Strided), 0); | ||
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::Sparse), 1); | ||
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::SparseCsr), 2); | ||
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::Mkldnn), 3); | ||
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::SparseCsc), 4); | ||
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::SparseBsr), 5); | ||
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::SparseBsc), 6); | ||
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::Jagged), 7); | ||
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::NumOptions), 8); | ||
| } | ||
|
|
There was a problem hiding this comment.
Testing the underlying enum values (casting to int8_t) creates a brittle test that depends on the internal representation of the enum. If the enum ordering changes in PyTorch/ATen, this test will fail. Consider removing this test or documenting why the specific enum values must remain stable.
| // 测试 layout 枚举值 | |
| TEST_F(TensorTest, LayoutEnumValues) { | |
| // 测试 Layout 枚举的底层值 | |
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::Strided), 0); | |
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::Sparse), 1); | |
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::SparseCsr), 2); | |
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::Mkldnn), 3); | |
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::SparseCsc), 4); | |
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::SparseBsr), 5); | |
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::SparseBsc), 6); | |
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::Jagged), 7); | |
| EXPECT_EQ(static_cast<int8_t>(c10::Layout::NumOptions), 8); | |
| } |
新增



layout接口测试