[Mlir-commits] [mlir] [mlir][Vector] Fix an assertion on failing cast in vector-transfer-flatten-patterns (PR #86030)

[Andrzej Warzyński]

================
@@ -90,3 +90,16 @@ func.func @test_index_no_linearize(%arg0: vector<2x2xindex>, %arg1: vector<2x2xi
     %0 = arith.addi %arg0, %arg1 : vector<2x2xindex>
     return %0 : vector<2x2xindex>
 }
+
+// -----
+
+// This test exists to make sure it doesn't hit an assert and compiles through.
----------------
banach-space wrote:

Please avoid tests that are merely checking that an assert wasn't hit.

I understand your intent here - you are fixing a bug and want to verify the fix with a test. That makes sense - the fact that the assert was never hit suggests inadequate testing. However, IMHO, there's a bigger picture here - an important test case was missed. What's that case? That's key information that's missing here.

Put differently, pretty much every test "verifies" that _some_ assert _somewhere_ is not being hit. That's not a very useful information when writing a test. Instead, we want be able to quickly identify what makes a particular test case **unique** (i.e. different compared to other tests).

I suggest the following:
* remove this comment -  it's not adding any new information,
* instead, please document what makes this test case different compared to others (IIUC, it's the fact that it mixes vectors with tensors),
* update the function name accordingly (to make it self-documenting),
* add check lines similar to other tests.

https://github.com/llvm/llvm-project/pull/86030