ChuanqiXu9 wrote: > I took a look at what sort of complexity unittest would entail here. I don't > think it's a good compromise complexity wise, it's a lot of boilerplate just > to test a few AST nodes are correctly linked.
But they are much easier to maintain than dumpped AST text. I feel things get pretty clear with unittests. And I don't think there are a lot of complexities. They indeed have some boilerplates. But I think it is worhty to make things get tested. Also the boilerplates can be improved by refactoring and it is not so hurting since they are tests. > > On the other hand, these AST tests don't look particularly out of place > compared to a lot of other AST tests we have. But that is not good. We should try to avoid that. > > However, I have seen that there are a lot of other related merging issues, so > I will leave the tangentially related tests for another MR. I am not sure what's your meaning. What's your intention of testing of this patch itself? And I think it is not maintainable or acceptable to test the dumpped text in the following patches. https://github.com/llvm/llvm-project/pull/80245 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
