On Thu, Feb 11, 2021, 8:08 PM Josef Šimánek <josef.sima...@gmail.com> wrote:
> čt 11. 2. 2021 v 15:27 odesílatel Matthias van de Meent > <boekewurm+postg...@gmail.com> napsal: > > > > On Wed, 10 Feb 2021 at 07:43, Bharath Rupireddy > > <bharath.rupireddyforpostg...@gmail.com> wrote: > > > > > > On Tue, Feb 9, 2021 at 6:02 PM Matthias van de Meent > > > <boekewurm+postg...@gmail.com> wrote: > > > > > Also, you can add this to the current commitfest. > > > > > > > > See https://commitfest.postgresql.org/32/2977/ > > > > > > > > On Tue, 9 Feb 2021 at 12:53, Josef Šimánek <josef.sima...@gmail.com> > wrote: > > > > > > > > > > OK, would you mind to integrate my regression test initial patch as > > > > > well in v3 or should I submit it later in a separate way? > > > > > > > > Attached, with minor fixes > > > > > > Why do we need to have a new test file progress.sql for the test > > > cases? Can't we add them into existing copy.sql or copy2.sql? Or do > > > you have a plan to add test cases into progress.sql for other progress > > > reporting commands? > > > > I don't mind moving the test into copy or copy2, but the main reason > > to put it in a seperate file is to test the 'copy' component of the > > feature called 'progress reporting'. If the feature instead is 'copy' > > and 'progress reporting' is part of that feature, then I'd put it in > > the copy-tests, but because the documentation of this has it's own > > docs page 'progress reporting', and because 'copy' is a subsection of > > that, I do think that this feature warrants its own regression test > > file. > > > > There are no other tests for the progress reporting feature yet, > > because COPY ... FROM is the only command that is progress reported > > _and_ that can fire triggers while running the command, so checking > > the progress view during the progress reported command is only > > feasable in COPY progress reporting. To test the other progress > > reporting views, we would need multiple sessions, which I believe is > > impossible in this test format. Please correct me if I'm wrong; I'd > > love to add tests for the other components. That will not be in this > > patchset, though. > > > > > IMO, it's better not add any new test file but add the tests to > existing files. > > > > In general I agree, but in some cases (e.g. new system component, new > > full-fledged feature), new test files are needed. I think that this > > could be one of those cases. > > I have split it since it should be the start of progress reporting > testing at all. If you better consider this as part of COPY testing, > feel free to move it to already existing copy testing related files. > There's no real reason to keep it separated if not needed. > +1 to move those test cases to existing copy test files.