Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check the parsed AST in the tests #60

Open
DCNick3 opened this issue May 29, 2021 · 1 comment
Open

Check the parsed AST in the tests #60

DCNick3 opened this issue May 29, 2021 · 1 comment

Comments

@DCNick3
Copy link

DCNick3 commented May 29, 2021

First of all, thanks for your great project, it helps me very much with parsing microsoft function definitions in their headers.

In spite of the last point of #59 I have some concerns regarding the test suite: it does not seem to check the resultant AST or generated code correctness.

I think it might be worthwhile to add such checks to all the tests. As of now I am considering replacing all the ast.show() and print(GnuCGenerator().visit(ast)) in the tests with checks against known good values.

The problem I see is that ast.show() results are incomplete (they lack attributes in some cases, for example), so it might not be the best idea to use it as-is.

What do you think regarding this issue? Maybe there is a better way to approach this issue?

@inducer
Copy link
Owner

inducer commented May 29, 2021

I'd be happy for any and all improvements to the tests. I think the round-trip tests I proposed in #59 are a reasonable thing to pursue, but I'm open to improvements. I don't currently use this package in any of my work, so I can't put too much work into it myself, but I'm happy to review contributions. Especially ones that are self-evidently correct, based on their tests. :)

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

No branches or pull requests

2 participants