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 1evLvH-0004HE-Mk for pgadmin-hackers@arkaria.postgresql.org; Mon, 12 Mar 2018 11:47:24 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1evLvG-00066a-6w for pgadmin-hackers@arkaria.postgresql.org; Mon, 12 Mar 2018 11:47:22 +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 1evLvF-00066L-LN for pgadmin-hackers@lists.postgresql.org; Mon, 12 Mar 2018 11:47:22 +0000 Received: from mail-ot0-x22c.google.com ([2607:f8b0:4003:c0f::22c]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1evLvC-0001hV-8m for pgadmin-hackers@postgresql.org; Mon, 12 Mar 2018 11:47:20 +0000 Received: by mail-ot0-x22c.google.com with SMTP id 108so14939572otv.3 for ; Mon, 12 Mar 2018 04:47:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=tU0JE5D+uXrNHLofmmqRF6SnaTY1KrYOTSRX6m3ObBQ=; b=ZTEyKDEIEB8tUbdasHzRZ1ihKZ90TtpRLqC65HL9pfM6H4M5zjxpNxLTfLu9fMpUYH FXEw+zOAosP1ule7HxIUYSYmHfG6NwzQ4oowFF24xpm0f4Lw57TTLfV38MVlHXONkST3 VAujW4616uNtTGuYf0/TUEaE+w8N/vZ68fvVtiv4EDRJcdCCF+CCq0IQc1VfuuZRHB5Q qsJUkeT+NzwwYwZCnQtlddqcd3bCoJrf19CymFXs1uIA+L7SUA6DqEDsbdcR2FDr2GJk Nrglolv5MaW46B4/5xXvfy3/PsO+HU3EPDcHcAxlE8MPLP7PZkdNgJVbdY1Bsb3C8+F5 O+PA== 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=tU0JE5D+uXrNHLofmmqRF6SnaTY1KrYOTSRX6m3ObBQ=; b=STlRcUzjMyGSCpL7DLuT3dJMYbemXOPxPt+m/mYZV6YNmNhd9CXRsNV3uHPpF13wzj teynbfiqf0vj497DzzMlH6L1R1Eqdjc/q1A7yDIw6O243B2kBWZu+8m4cKXXqzem4kUE 3RPQdSbG9T6P6dvQkZXnSUxv8voDTzl04G4qsrdV5m1LrIe6AmoKLk5/yXaajg9eEg12 AUNjvxdghvQPkzzzyJByD0bSoBB03kncKPawqqFGkkPLQpOfAmcCLCzGwTRVYee9kP7X AWp35JXj1z/BY8+LbkP6CMcAk/9PK+QZEwOVq6gp//eYmgXcUlzhtY49uATJx8npOVh9 8mEA== X-Gm-Message-State: AElRT7Hw6shOCiJ8Dl2a0yNAgmHL0wkAdQs/C2EeyiJ7s7B9sCOgW2uC XsunQ58mF2Vs70iRtsKhc1B808fEa3s8o2QQigSqaA== X-Google-Smtp-Source: AG47ELvqmCB4qnpApdUkNP2G36EHJITV8NpCsn6i3sQkgVUwmWgEhNcTBOl5v/aUva+SnEXE2m3Z+uscpjF7ftGp1ag= X-Received: by 10.157.11.178 with SMTP id 47mr4866044oth.277.1520855237161; Mon, 12 Mar 2018 04:47:17 -0700 (PDT) MIME-Version: 1.0 Received: by 10.74.203.154 with HTTP; Mon, 12 Mar 2018 04:47:16 -0700 (PDT) In-Reply-To: References: From: Khushboo Vashi Date: Mon, 12 Mar 2018 17:17:16 +0530 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: Dave Page Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary="001a113e20e82a8364056735b420" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk --001a113e20e82a8364056735b420 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Mon, Mar 12, 2018 at 5:15 PM, Dave Page wrote: > 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 >>>>> 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 =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 --verbos= e >>>> --format=3Dc --blobs "é" >>>> >>> >>> I can reproduce this issue only with notification dialogue (which I hav= e >> 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 ca= n >>> 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 th= is >>> might need some config parameters for the tests to specify the pg_* uti= lity >>> 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 th= e >>> 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 databa= se >>> (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 bac= kup >>> 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 don= e >> 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 th= e > 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 > --001a113e20e82a8364056735b420 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Mon, Mar 12, 2018 at 5:15 PM, Dave Page <dpage@pgadmin.org><= /span> wrote:
Hi

On M= on, Mar 12, 2018 at 2:00 AM, Khushboo Vashi <khushboo.vash= i@enterprisedb.com> wrote:

Hi Dave,

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

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

Plea= se 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 comp= lete 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 sam= e which will cover this.

Interesting that you fix these together, as together they also exhibit ano= ther bug :-). Backing up the =C3=A9=C2=A0d= atabase displays the following command:

/usr/local/pgsql/bin/pg_dump --file "/Users/dpage= /foo.bak" --host "localhost" --port "5432" --usern= ame "postgres" --no-password --verbose --format=3Dc --blobs "= ;&#233;"=C2=A0
I can reproduce th= is issue only with notification dialogue (which I have fixed in the attache= d patch) not with the details dialogue. Please refer the screenshots for th= e same.

Huh,= interesting. What Python version were you using? I had 2.7.
=C2=A0
PY 2.7 = and PY 3.5=C2=A0
=
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 w= e can backup and restore a database correctly, and ensure that the displaye= d 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 s= hould 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.=C2=A0

I'd suggest maybe having a = feature test that opens the prefs, sets the appropriate path, then runs a b= ackup, waits for it to finish, checks the process monitor output, then rest= ores 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_dum= p/pg_restore, just that something expected was restored). We should use a (= partial) database name and backup filename from the advanced test config fi= le, and I think both should default to some interesting non-ASCII strings t= o ensure quoting works.
I w= as 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 f= or 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 te= st cases.

So= unds good. Let's try to keep the feature testing minimal, and expand th= e coverage with unit tests.

Sure.=C2=A0
=
Thanks!

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

Ent= erpriseDB UK: htt= p://www.enterprisedb.com
The Enterprise PostgreSQL Company

--001a113e20e82a8364056735b420--