> 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. Robins Tharakan
