srielau commented on PR #53530: URL: https://github.com/apache/spark/pull/53530#issuecomment-3761345919
> > should we add an overload of `def getCursorState` that accepts a `CursorReference`? This code snippet repeats 3 times. > > The debate between sql scripts with goldenfiles and scala unittests is very much philosophical. > > Personbally I find a sql.out file much more readable than a scala file. > > More importantly though I have very much hostile to unit-tests which only tests certain phases (e.g. parsing). > > They break due to design changes, and they do not test outcome. And outcomes are what matters. > > Golden files, of course have teh downside risk, that verytime they are re-generated teh user has to actually look at the diff. > > If there are strong feelings to convert the SQL scripts to Scala, I can certainly do that. Curiously, in another PR I was asked to do the inverse.... Test suite has been turned into Scala. Some further reflection: Turns out Cursor is terribly bad (or my prompts) about preventing drift when regenerating golden files. So this exercise did in fact flush out bugs. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
