On Tue, Jan 14, 2025 at 11:48:28AM -0800, Noah Misch wrote: > I misunderstood, and I was mistaken to see this as a bug fix. The > isolationtester is acting per its definition, and this would be a definition > change. Do others have opinions on the merits of today's definition vs. the > proposed definition?
I agree that Michail's case with the handling of the markers is kind of strange in the case he has reported. Anyway, do you think that it would be a good idea to change that knowing for how long we've relied on isolationtester to do things the way they are? If we do it only on HEAD, it would make the back-patching of newly-added tests for bug fixes potentially more complex to deal with. If applying the new definition across all the stable branches, this could impact something outside code :/ > I'd need to review the motivating test to form my own opinion on whether the > new definition makes it easier to write tests. I'm optimistic that it does > make things easier, because I think one can get the old behavior by defining > two steps with the same commands. Thanks for the patch. You could bundle > this in the thread that wants this to stabilize that thread's CI results. If you can get a backward-compatible behavior, well, no objections from here, I guess. > FYI, I prefer the convention of ending each step name with the session number, > i.e. s/after/after1/ here. The first isolation specs used that style. I > mildly prefer it over the newer alternative styles, and I strongly prefer > styles that convey the session identity over styles that don't. No need to > send a new patch version, though. +1. Including the session number in the steps makes debugging easier. -- Michael
signature.asc
Description: PGP signature
