junrushao commented on PR #13190: URL: https://github.com/apache/tvm/pull/13190#issuecomment-1290621348
Thanks for the great contribution! I recognize that most of the TIR syntax have been covered by the IRBuilder tests accordingly, for example, `T.block` has been carefully tested in IRBuilder. This decoupling approach is great because it largely simplifies testing effort, avoids end-to-end integration tests and favors more fine grained unittests. Therefore, given this PR is tested against all the nuanced cases not covered by the IRBuilder tests, so i would say the test coverage is good enough. On the other hand, I do think it’s desirable for us to see some end-to-end tests to make sure the parser is functioning properly, especially when e2e tests are running super fast for in the parser. A couple of test cases I would love to see in a follow-up PR: - `1 + 1` is not simplified to `2`, but honestly translated to `tvm.tir.Add` - `T.Buffer()` in function signature is correctly parsed - Parser error is properly reported in IRBuilder calls, for example, calling an non-existing intrinsic. Would be great if we could have a follow-up PR for such tests! Thanks a lot! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
