Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1evLu2-0004Cr-HD for pgadmin-hackers@arkaria.postgresql.org; Mon, 12 Mar 2018 11:46:06 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1evLu1-0005ZO-4h for pgadmin-hackers@arkaria.postgresql.org; Mon, 12 Mar 2018 11:46:05 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.89) (envelope-from ) id 1evLu0-0005ZE-OL for pgadmin-hackers@lists.postgresql.org; Mon, 12 Mar 2018 11:46:05 +0000 Received: from mail-wr0-x241.google.com ([2a00:1450:400c:c0c::241]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1evLts-0001gw-OO for pgadmin-hackers@postgresql.org; Mon, 12 Mar 2018 11:46:03 +0000 Received: by mail-wr0-x241.google.com with SMTP id v65so15330351wrc.11 for ; Mon, 12 Mar 2018 04:45:56 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin-org.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=CroXWVF3bgEzlYthvwqThUBr6FJ4s+RY26HLe6QFFk0=; b=XNgcoqTnQvsepNvMXCAupl67XQzydUBNhF73C3sD2oees94//WGevV8bUqrJgnVO+/ 7pJe0WqC4mry6N7TeOi6Uzdb3rSupH57x6tuEwJsxUvlX4T4oJlHKis1OXeML2hWjrVO ox5jPStvYRqHJ+az3wt3OpI7op7UNhX8za1MC1FWISQyqoM+TIlSDvWu5eKOUFYANgum Oxfl2ByIrewa1L+Uoee+Y0TygzZm5Uz3iuZ3ihHsiq+Ev22FFk+CJ7xFOt/UGW8DjrpU sV6od4bWYxmViVcOz/PWROVzkQgIGhPj8tT6e+EleKjhkYoV3fInaBs03Xm7Koy5MS+k Wamg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=CroXWVF3bgEzlYthvwqThUBr6FJ4s+RY26HLe6QFFk0=; b=a9UvSSYRzA1Qv8THwBmetoQ+MRJ83fZXfiq+gezUEDw5Kvm274SyXwfnmfa1zkqVjH 88k76o4JE8PfN9z5HRguEK1C5HCwy7hoFSJbBbj8o2RIZ7IfpYpXpAP3qLReJFwQeMGG dB64wfwz2lXi9Ds/3z0Tp2dnK/rJrd9S+E+0m4fqZ1lNr+d/CcnwxzXGQ/ISxdY+1nGv 0so+Zc6Is4JU181ciBfGCxEh09zkBn7p18dN1TSH4il1GM1wJA1pm04m6xJD8Ntxn5Hp AvE6S3g1N7+iOpVAJQHq2+2B8zJhXj68qe9vPbwlFNl/oK6prT4nJy+JtWUKtuIyqEak /LFw== X-Gm-Message-State: AElRT7FdZB2gBwsPURnrACmFiXCcEGBv6stYGBXhVduMEzylnxZSEnyP +0DMgDT5kWqKBVxvPFLOpkXpdlCRREYd0kI+t4X2DA== X-Google-Smtp-Source: AG47ELsa74DiyoEEABF7Mb6LbxQ5Jo3oOks5J4lpfV+6Vq0eDdtk7KHmIbFNmspJRubPbIwuFjnux5yv0YC6Dvndwvo= X-Received: by 10.223.129.80 with SMTP id 74mr4885323wrm.190.1520855154797; Mon, 12 Mar 2018 04:45:54 -0700 (PDT) MIME-Version: 1.0 Received: by 10.28.109.7 with HTTP; Mon, 12 Mar 2018 04:45:53 -0700 (PDT) In-Reply-To: References: From: Dave Page Date: Mon, 12 Mar 2018 07:45:53 -0400 Message-ID: Subject: =?UTF-8?Q?Re=3A_=5BpgAdmin4=5D=5BPatch=5D=3A_RM_=232963_=2D_Backup_database=2C_R?= =?UTF-8?Q?estore_database_and_Maintenance_Database_failed_for_=C3=A9_objec?= =?UTF-8?Q?t=2E?= To: Khushboo Vashi Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary="94eb2c05f2e441cc47056735af63" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --94eb2c05f2e441cc47056735af63 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi On Mon, Mar 12, 2018 at 2:00 AM, Khushboo Vashi < khushboo.vashi@enterprisedb.com> wrote: > > Hi Dave, > > On Fri, Mar 9, 2018 at 9:09 PM, Dave Page wrote: > >> Hi >> >> On Fri, Mar 9, 2018 at 3:32 PM, Dave Page wrote: >> >>> Hi >>> >>> On Fri, Mar 9, 2018 at 3:54 AM, Khushboo Vashi < >>> khushboo.vashi@enterprisedb.com> wrote: >>> >>>> Hi, >>>> >>>> Please find the attached patch to fix below issues: >>>> >>>> 1. #2963 - Backup database, Restore database and Maintenance Database >>>> failed for =C3=A9 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 case= s >>>> 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 =C3=A9 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=3Dc --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. > 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 thi= s >> might need some config parameters for the tests to specify the pg_* util= ity >> 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 databas= e >> (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 back= up >> 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 a= s > 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. Thanks! --=20 Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company --94eb2c05f2e441cc47056735af63 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

On Mon, Mar 12, 2018 at 2:00 AM, Khushboo Vashi <= ;khush= boo.vashi@enterprisedb.com> wrote:

Hi Dave,

On Fr= i, Mar 9, 2018 at 9:09 PM, Dave Page <dpage@pgadmin.org> wro= te:
Hi

On Fri, Mar 9, 2018 at 3:= 32 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Fri, Mar 9, 2018 at 3:54 AM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Hi,

<= span>
Please find the attached patch to fix below issues:
1. #2963 - Backup database, Restore database and Maintenance Da= tabase failed for =C3=A9 object
2. #3157 - Process viewer doesn&#= 39;t show complete command executed.

Test cases ar= e not included for these fixes as we don't have test cases for these mo= dules (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 al= so exhibit another bug :-). Backing up the =C3=A9= =C2=A0database displays the following command:

<= div>/usr/local/pgsql/bin/pg_dump --file &quo= t;/Users/dpage/foo.bak" --host "localhost" --port "5432= " --username "postgres" --no-password --verbose --format=3Dc= --blobs "&#233;"=C2=A0

I ca= n reproduce this issue only with notification dialogue (which I have fixed = in the attached patch) not with the details dialogue. Please refer the scre= enshots for the same.

Huh, interesting. What Python version were you using? I had 2.7.=C2=A0
Also, what tests can we add for b= ackup/restore? We have nothing at all at the moment, and it is pretty troub= lesome. I'd like to ensure that we can backup and restore a database co= rrectly, 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 a= nd generic). I guess this might need some config parameters for the tests t= o specify the pg_* utility paths for each server.=C2=A0

I'd suggest maybe having a feature test that opens the prefs, set= s 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 res= tored 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 expe= cted was restored). We should use a (partial) database name and backup file= name 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 c= ases 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 afte= r 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.

Thanks!

-- <= br>
Dave P= age
Blog: http= ://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterpri= sedb.com
The Enterprise PostgreSQL Company
--94eb2c05f2e441cc47056735af63--