However, before it can get committed, I think this set of tests needs streamlining. It does seem to me valuable, but I think it's wasteful in terms of runtime to create so many roles, do just one thing with them, and then drop them. I recommend consolidating some of the tests. For example:
Generally, I think that the tests which return a syntax error are of limited value and should probably be dropped. That is unlikely to get broken by accident. If the syntax error is being thrown by something outside of bison proper, that's probably worth testing. But I think that testing random syntax variations is a pretty low-value proposition.
Thanks Robert.
Although the idea of being repetitive was just about trying to make tests simpler to infer for the next person, but I guess this example was obviously an overkill. Point taken, would correct and revert with an updated patch.
However, the other aspect that you mention, I am unsure if I understand correctly. Do you wish that syntactical errors not be tested? If so, probably you're referring to tests such as the one below, and then I think it may get difficult at times to bifurcate how to chose which tests to include and which to not. Can I assume that all errors that spit an error messages with 'syntax error' are to be skipped, probably that'd be an easy test for me to know what you'd consider important?
+ALTER ROLE regress_rol_rol18 WITH ENCRYPTED UNENCRYPTED PASSWORD 'abc'; +ERROR: syntax error at or near "UNENCRYPTED" +LINE 1: ALTER ROLE regress_rol_rol18 WITH ENCRYPTED UNENCRYPTED PASS...
Personally, I think all tests are important. Unless there is a clear understanding that aiming for 100% code-coverage isn't the goal, I think all tests are important, syntactical or otherwise. Its possible that not all code is reachable (therefore testable) but the vision generally remains at 100%.
Do let me know your view on this second point, so that I can remove these tests if so required.