Community
Participate
Working Groups
Codan test cases are executed correctly even if there are syntax errors in the test code.
What does this mean? What test cases? What means executed correctly? Please provide example of source code expected output and actual output.
It means the test case can succeed even if the code fails to parse. I think that's not a good default behaviour, because it can mask errors in the test code that cause the test to not actually exercise the desired Codan codepath. I think that by default, if the code fails to parse, the testcase should fail. For the less common case where you want to test Codan behaviour on code that fails to parse, there can be a opt-in mechanism, similar to CPPASTNameBase.sAllowRecursionBindings.
Here's a possible outline of how a fix could work: - Extend CodanRunner.processResource() and related functions to allow the caller to pass in an ICheckerInvocationContext. This way, the test suite can pass in its own context, and the context can serve as a channel for checkers to communicate with the test suite. - Create a type for storing a syntax error count in the context. CheckerTestCase.runCodan() can create an instance of this object and stash it in the context (using ICheckerInvocationContext.add()) before passing the context to processResource(). - In AbstractIndexAstChecker, before calling processAst(), we can check the context to see if it contains an object of this type. If so, we interpret that as a request to tell the context about the number of syntax errors. We count them using CPPVisitor.getProblems(ast).length, and store it in the object. - After processResource(), CheckerTestCase.runCodan() checks the context for the syntax error count and fails the test if the count is not zero.
Looks like total overkill to me. Use code coverage to make sure tests are actually covering the checker code, if you don't see expected coverages maybe test is wrong.
I think it would have value while writing the test case to have quick feedback of why things are not working when there are syntax errors. Even with code coverage, knowing that the branch is not taken is not as quick of a feedback than just failing the test. It also doesn't have to be too complicated to do I think. I just modified CheckerTestCase.checkNoErrors with: ITranslationUnit tu = (ITranslationUnit) currentCElem; IASTTranslationUnit ast = tu.getAST(); if (ast.getPreprocessorProblemsCount() > 0) { fail("Found preprocessor problems"); } IASTProblem[] problems = tu.isCLanguage() ? CVisitor.getProblems(ast) : CPPVisitor.getProblems(ast); if (problems.length > 0) { fail("Found ast problems:" + problems[0].getMessage()); } Of course this can be refined with better output, it's just a proof of concept. The getAST call means reparsing the file again but the snippet are very small so I don't think it matters either way and it means not having to change anything else (CodanRunner, etc).
