Re: pgAdmin IV : Unittest modular patch

Hi

On Mon, Aug 1, 2016 at 10:54 AM, Navnath Gadakh
<navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
> Hi Dave,
>
> On Mon, Aug 1, 2016 at 3:17 PM, Dave Page <dave(dot)page(at)enterprisedb(dot)com> wrote:
>> On Thu, Jul 28, 2016 at 8:12 AM, Navnath Gadakh
>> <navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>>> Thanks Dave!
>>>
>>> On Wed, Jul 27, 2016 at 8:12 PM, Dave Page <dave(dot)page(at)enterprisedb(dot)com> wrote:
>>>> Ahh, that explains it. Nice catch! It's working in both server and
>>>> desktop modes now :-).
>>>>
>>>> I've committed the patch, but can you please look at the following issues:
>>>>
>>>> 1) When testing SMTP, the encoded password used to connect to the
>>>> server is displayed in both the logs and stdout (the "send: 'AUTH
>>>> PLAIN ....'" lines). Can you catch that, and replace it with *'s
>>>> please? Sooner or later someone will reveal a password otherwise.
> Could you please send me your's regression.log file. While
> testing SMTP I didn't see encoded password displayed on both logs and
> stdout.

Here's an extract from stdout (with the encoded password replaced with
xxxxxx). I don't actually see the same in regression.log, but that may
be because of the log settings I'm using - we should filter the same
from there though to be safe:

...
This function checks reset password functionality. (TestCase for
Validating Valid_Email) ... send: 'ehlo
piranha.ox.uk.enterprisedb.com\r\n'
reply: '250-smtp.gmail.com at your service, [194.73.70.210]\r\n'
reply: '250-SIZE 35882577\r\n'
reply: '250-8BITMIME\r\n'
reply: '250-AUTH LOGIN PLAIN XOAUTH2 PLAIN-CLIENTTOKEN OAUTHBEARER XOAUTH\r\n'
reply: '250-ENHANCEDSTATUSCODES\r\n'
reply: '250-PIPELINING\r\n'
reply: '250-CHUNKING\r\n'
reply: '250 SMTPUTF8\r\n'
reply: retcode (250); Msg: smtp.gmail.com at your service, [194.73.70.210]
SIZE 35882577
8BITMIME
AUTH LOGIN PLAIN XOAUTH2 PLAIN-CLIENTTOKEN OAUTHBEARER XOAUTH
ENHANCEDSTATUSCODES
PIPELINING
CHUNKING
SMTPUTF8
send: 'AUTH PLAIN xxxxxx\r\n'
reply: '235 2.7.0 Accepted\r\n'
reply: retcode (235); Msg: 2.7.0 Accepted
send: u'mail FROM:<no-reply(at)localhost> size=1122\r\n'
reply: '250 2.1.0 OK v26sm44372678pfi.41 - gsmtp\r\n'
reply: retcode (250); Msg: 2.1.0 OK v26sm44372678pfi.41 - gsmtp
send: u'rcpt TO:<dpage(at)pgadmin(dot)org>\r\n'
reply: '250 2.1.5 OK v26sm44372678pfi.41 - gsmtp\r\n'
reply: retcode (250); Msg: 2.1.5 OK v26sm44372678pfi.41 - gsmtp
send: 'data\r\n'
reply: '354 Go ahead v26sm44372678pfi.41 - gsmtp\r\n'
reply: retcode (354); Msg: Go ahead v26sm44372678pfi.41 - gsmtp
data: (354, 'Go ahead v26sm44372678pfi.41 - gsmtp')
send: 'Content-Type: multipart/mixed;
boundary="===============5947340609837468105=="\r\nMIME-Version:
1.0\r\nSubject: Password reset instructions for pgAdmin 4\r\nFrom:
no-reply(at)localhost\r\nTo: dpage(at)pgadmin(dot)org\r\nDate: Mon, 01 Aug 2016
11:18:01 +0100\r\nMessage-ID:
<20160801101758(dot)1954(dot)48817(at)piranha(dot)ox(dot)uk(dot)enterprisedb(dot)com>\r\n\r\n--===============5947340609837468105==\r\nContent-Type:
multipart/alternative;\r\n
boundary="===============4573645557236154244=="\r\nMIME-Version:
1.0\r\n\r\n--===============4573645557236154244==\r\nContent-Type:
text/plain; charset="utf-8"\r\nMIME-Version:
1.0\r\nContent-Transfer-Encoding: 7bit\r\n\r\nClick the link below to
reset your password:\r\n\r\nhttp://localhost/reset/WyIxIiwiYTY2ODY0ZTY0NzlmMzM1NTNhYTc0NzUwYzEwNDc5ZmQiXQ.CoCzVg.FE0MhiGBwOgVWTcX8mB3Xgq54yw\r\n--===============4573645557236154244==\r\nContent-Type:
text/html; charset="utf-8"\r\nMIME-Version:
1.0\r\nContent-Transfer-Encoding: 7bit\r\n\r\n<p><a
href="http://localhost/reset/WyIxIiwiYTY2ODY0ZTY0NzlmMzM1NTNhYTc0NzUwYzEwNDc5ZmQiXQ.CoCzVg.FE0MhiGBwOgVWTcX8mB3Xgq54yw">Click
here to reset your
password</a></p>\r\n--===============4573645557236154244==--\r\n\r\n--===============5947340609837468105==--\r\n.\r\n'
reply: '250 2.0.0 OK 1470046685 v26sm44372678pfi.41 - gsmtp\r\n'
reply: retcode (250); Msg: 2.0.0 OK 1470046685 v26sm44372678pfi.41 - gsmtp
data: (250, '2.0.0 OK 1470046685 v26sm44372678pfi.41 - gsmtp')
send: 'quit\r\n'
reply: '221 2.0.0 closing connection v26sm44372678pfi.41 - gsmtp\r\n'
reply: retcode (221); Msg: 2.0.0 closing connection v26sm44372678pfi.41 - gsmtp
ok
runTest (pgadmin.browser.server_groups.servers.tests.test_server_put.ServerUpdateTestCase)
...

These are the relevant settings I'm using:

# Debug mode
DEBUG = True

# App mode
SERVER_MODE = True

# Log
CONSOLE_LOG_LEVEL = DEBUG
FILE_LOG_LEVEL = DEBUG

# Mail server settings
MAIL_SERVER = 'smtp.gmail.com'
MAIL_PORT = 465
MAIL_USE_SSL = True
MAIL_USERNAME = 'dave(dot)page(at)enterprisedb(dot)com'
MAIL_PASSWORD = 'xxxxxx'

>>>>
>>>> 2) Is the test count in the summary correct when you have multiple
>>>> servers? I think it might be resetting between servers.
>>> In the current code runTest() function calls only one time for
>>> single/multiple server/s that's why it's showing same count for both
>>> (single & multiple).
>>> Question: What exactly we are looking for, only count? Or we need
>>> any more logs like for adding database, it should display in which
>>> server (PG/PPAS) its adding database if this is the case we need a
>>> feasibility check.
>>
>> Right now I would like to see the summary updated so it outputs useful
>> (and correct) information - for example, it could say something like:
>>
>> =====
>> Testing completed:
>>
>> PostgreSQL 9.4: 23 tests passed, 0 tests failed
>> EPAS 9.5: 25 tests passed, 2 tests failed (foo_test, bar_test)
>> =====
>>
>> where PostgreSQL 9.4 and EPAS 9.5 are the two servers configured in
>> test_config.py and foo_test and bar_test are the names of the tests
>> that failed.
> Thanks for additional info.
>>
>>>> 3) Please ensure that test databases that are created are also dropped
>>>> again. I'm getting tired of deleting them manually!
>>> I already added delete databases in tearDown () function, But
>>> yeah, I found two test cases where this code missed. I will add this
>>> in the next patch.
>>
>> Thanks.
>>
>>>>
>>>> Thanks, Dave.
>>>>
>>>> On Wed, Jul 27, 2016 at 1:19 PM, Navnath Gadakh
>>>> <navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>>>>> Hi Dave,
>>>>> We have reproduced the issue you mentioned in the previous
>>>>> email. We need to skip the login related test cases when SERVER_MODE =
>>>>> False in the web/config_local.py.
>>>>>
>>>>> Please find the revised patch for the same.
>>>>>
>>>>> Thanks!
>>>>>
>>>>>
>>>>> On Fri, Jul 22, 2016 at 9:48 PM, Dave Page <dave(dot)page(at)enterprisedb(dot)com> wrote:
>>>>>> Hi
>>>>>>
>>>>>> On Fri, Jul 22, 2016 at 2:22 PM, Navnath Gadakh
>>>>>> <navnath(dot)gadakh(at)enterprisedb(dot)com> wrote:
>>>>>>> Hi Dave,
>>>>>>>
>>>>>>> Please find the attached patch for pgAdmin4 unittest(Support of API
>>>>>>> testing with different servers i.e. PG and PPAS).
>>>>>>>
>>>>>>> This patch includes-
>>>>>>>
>>>>>>> Test framework support API testing with multiple server for this we need to
>>>>>>> modify test_config.json(for user it’s test_config.json.in) and
>>>>>>> test_advanced_config.json(for user it’s test_advanced_config.json.in).
>>>>>>> Server details of PG and PPAS are included in both .in files.
>>>>>>>
>>>>>>> Removed the logic of logging in the test client on each test scenario(As
>>>>>>> per Khushboo's comment in previous email). We need this logic in test cases
>>>>>>> under ‘browser/tests/’ as for test scenarios like change password and
>>>>>>> invalid login test cases as test client should be logged out first. So, as
>>>>>>> per this the code is slightly modified in ‘browser/tests/’.
>>>>>>
>>>>>> OK in principle, but it doesn't seem to work:
>>>>>>
>>>>>> This function will add the server under default server group. (Default
>>>>>> Server Node url) ... ok
>>>>>>
>>>>>> ======================================================================
>>>>>> FAIL: runTest (pgadmin.browser.tests.test_reset_password.ResetPasswordTestCase)
>>>>>> This function checks reset password functionality. (TestCase for
>>>>>> Validating Empty Email)
>>>>>> ----------------------------------------------------------------------
>>>>>> Traceback (most recent call last):
>>>>>> File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/tests/test_reset_password.py",
>>>>>> line 44, in runTest
>>>>>> 'utf-8'))
>>>>>> AssertionError: 'Recover pgAdmin 4 Password' not found in u'<!DOCTYPE
>>>>>> HTML PUBLIC "-//W3C//DTD HTML 3.2
>>>>>> Final//EN">\n<title>Redirecting...</title>\n<h1>Redirecting...</h1>\n<p>You
>>>>>> should be redirected automatically to target URL: <a href="/">/</a>.
>>>>>> If not click the link.'
>>>>>>
>>>>>> ======================================================================
>>>>>> FAIL: runTest (pgadmin.browser.tests.test_reset_password.ResetPasswordTestCase)
>>>>>> This function checks reset password functionality. (TestCase for
>>>>>> Validating Invalid_Email)
>>>>>> ----------------------------------------------------------------------
>>>>>> Traceback (most recent call last):
>>>>>> File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/tests/test_reset_password.py",
>>>>>> line 44, in runTest
>>>>>> 'utf-8'))
>>>>>> AssertionError: 'Recover pgAdmin 4 Password' not found in u'<!DOCTYPE
>>>>>> HTML PUBLIC "-//W3C//DTD HTML 3.2
>>>>>> Final//EN">\n<title>Redirecting...</title>\n<h1>Redirecting...</h1>\n<p>You
>>>>>> should be redirected automatically to target URL: <a href="/">/</a>.
>>>>>> If not click the link.'
>>>>>>
>>>>>> ======================================================================
>>>>>> FAIL: runTest (pgadmin.browser.tests.test_reset_password.ResetPasswordTestCase)
>>>>>> This function checks reset password functionality. (TestCase for
>>>>>> Validating Valid_Email)
>>>>>> ----------------------------------------------------------------------
>>>>>> Traceback (most recent call last):
>>>>>> File "/Users/dpage/git/pgadmin4/web/pgadmin/browser/tests/test_reset_password.py",
>>>>>> line 44, in runTest
>>>>>> 'utf-8'))
>>>>>> AssertionError: 'Recover pgAdmin 4 Password' not found in u'<!DOCTYPE
>>>>>> HTML PUBLIC "-//W3C//DTD HTML 3.2
>>>>>> Final//EN">\n<title>Redirecting...</title>\n<h1>Redirecting...</h1>\n<p>You
>>>>>> should be redirected automatically to target URL: <a href="/">/</a>.
>>>>>> If not click the link.'
>>>>>>
>>>>>> ----------------------------------------------------------------------
>>>>>> Ran 26 tests in 30.109s
>>>>>>
>>>>>> FAILED (failures=3)
>>>>>>
>>>>>> It's very far from obvious what's wrong here (/reset looks just fine
>>>>>> to me). How can we get more useful info out of the framework?
>>>>>>
>>>>>> --
>>>>>> Dave Page
>>>>>> VP, Chief Architect, Tools & Installers
>>>>>> EnterpriseDB: http://www.enterprisedb.com
>>>>>> The Enterprise PostgreSQL Company
>>>>>>
>>>>>> Blog: http://pgsnake.blogspot.com
>>>>>> Twitter: @pgsnake
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Thanks,
>>>>> Navnath Gadakh
>>>>> Software Engineer
>>>>> EnterpriseDB Corporation
>>>>> Mobile: +91 9975389878
>>>>
>>>>
>>>>
>>>> --
>>>> Dave Page
>>>> VP, Chief Architect, Tools & Installers
>>>> EnterpriseDB: http://www.enterprisedb.com
>>>> The Enterprise PostgreSQL Company
>>>>
>>>> Blog: http://pgsnake.blogspot.com
>>>> Twitter: @pgsnake
>>>
>>>
>>>
>>> --
>>> Thanks,
>>> Navnath Gadakh
>>> Software Engineer
>>> EnterpriseDB Corporation
>>> Mobile: +91 9975389878
>>
>>
>>
>> --
>> Dave Page
>> VP, Chief Architect, Tools & Installers
>> EnterpriseDB: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>>
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>
>
>
> --
> Thanks,
> Navnath Gadakh
> Software Engineer
> EnterpriseDB Corporation
> Mobile: +91 9975389878

--
Dave Page
VP, Chief Architect, Tools & Installers
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake