On Mon, Mar 12, 2018 at 5:15 PM, Dave Page <dp...@pgadmin.org> wrote:
> Hi > > On Mon, Mar 12, 2018 at 2:00 AM, Khushboo Vashi < > khushboo.va...@enterprisedb.com> wrote: > >> >> Hi Dave, >> >> On Fri, Mar 9, 2018 at 9:09 PM, Dave Page <dp...@pgadmin.org> wrote: >> >>> Hi >>> >>> On Fri, Mar 9, 2018 at 3:32 PM, Dave Page <dp...@pgadmin.org> wrote: >>> >>>> Hi >>>> >>>> On Fri, Mar 9, 2018 at 3:54 AM, Khushboo Vashi < >>>> khushboo.va...@enterprisedb.com> wrote: >>>> >>>>> Hi, >>>>> >>>>> Please find the attached patch to fix below issues: >>>>> >>>>> 1. #2963 - Backup database, Restore database and Maintenance Database >>>>> failed for é object >>>>> 2. #3157 - Process viewer doesn't show complete command executed. >>>>> >>>>> Test cases are not included for these fixes as we don't have test >>>>> cases for these modules (backup, restore, maintenance). >>>>> I will create one separate RM for the same which will cover this. >>>>> >>>> >>>> Interesting that you fix these together, as together they also exhibit >>>> another bug :-). Backing up the é database displays the following >>>> command: >>>> >>>> /usr/local/pgsql/bin/pg_dump --file "/Users/dpage/foo.bak" --host >>>> "localhost" --port "5432" --username "postgres" --no-password --verbose >>>> --format=c --blobs "é" >>>> >>> >>> I can reproduce this issue only with notification dialogue (which I have >> fixed in the attached patch) not with the details dialogue. Please refer >> the screenshots for the same. >> > > Huh, interesting. What Python version were you using? I had 2.7. > > PY 2.7 and PY 3.5 > Also, what tests can we add for backup/restore? We have nothing at all at >>> the moment, and it is pretty troublesome. I'd like to ensure that we can >>> backup and restore a database correctly, and ensure that the displayed >>> commands are what we expect and that we get valid output from >>> pg_dump/pg_restore (though, it may change from PG version to PG version, so >>> maybe we should just check for something small and generic). I guess this >>> might need some config parameters for the tests to specify the pg_* utility >>> paths for each server. >>> >>> I'd suggest maybe having a feature test that opens the prefs, sets the >>> appropriate path, then runs a backup, waits for it to finish, checks the >>> process monitor output, then restores the same backup to a new database, >>> checking the process monitor output again, and then checking that the >>> restored database contains at least one object from the original database >>> (we don't need to check all of pg_dump/pg_restore, just that something >>> expected was restored). We should use a (partial) database name and backup >>> filename from the advanced test config file, and I think both should >>> default to some interesting non-ASCII strings to ensure quoting works. >>> >> I was thinking of writing the unit test cases for the processes.py file >> as all the major functionalities for backup/restore/maintenance jobs done >> by this file, but by this we can not achieve the front-end string >> validation esp for non-ASCII strings. >> So, I am thinking of writing feature tests (as you have suggested) first >> and after that if needed I will write unit test cases. >> > > Sounds good. Let's try to keep the feature testing minimal, and expand the > coverage with unit tests. > > Sure. > Thanks! > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
