Skip to content

Conversation

@youge325
Copy link
Contributor

@youge325 youge325 commented Jan 4, 2026

新增 layout 接口测试
image
image

Copilot AI review requested due to automatic review settings January 4, 2026 07:44
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment on lines +235 to +266

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");
Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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");

Copilot uses AI. Check for mistakes.
Comment on lines +218 to +231
// 测试 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);
}

Copy link

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
// 测试 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);
}

Copilot uses AI. Check for mistakes.
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.

1 participant