Re: [pgadmin4][Patch]: Test cases for the backup module

Hi Khushboo,

We reviewed the patch and it is very nice to see some more coverage in this
area. Good job :D

We passed the tests through our CI the feature tests are not passing, but
the linter fails:

./pgadmin/feature_tests/pg_utilities_backup_test.py:37: [E501] line
too long (91 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:265>
./pgadmin/feature_tests/pg_utilities_backup_test.py:53: [E501] line
too long (104 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:266>
./pgadmin/feature_tests/pg_utilities_backup_test.py:59: [E501] line
too long (85 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:267>
./pgadmin/feature_tests/pg_utilities_backup_test.py:62: [E501] line
too long (96 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:268>
./pgadmin/feature_tests/pg_utilities_backup_test.py:63: [E501] line
too long (91 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:269>
./pgadmin/feature_tests/pg_utilities_backup_test.py:70: [E501] line
too long (118 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:270>
./pgadmin/tools/backup/tests/test_backup_message.py:37: [E121]
continuation line under-indented for hanging indent
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:271>
./pgadmin/tools/backup/tests/test_backup_message.py:48: [E122]
continuation line missing indentation or outdented
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:272>
./pgadmin/tools/backup/tests/test_backup_message.py:49: [E251]
unexpected spaces around keyword / parameter equals
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:273>
./pgadmin/tools/backup/tests/test_backup_message.py:49: [E251]
unexpected spaces around keyword / parameter equals
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:274>
./pgadmin/tools/backup/tests/test_backup_message.py:51: [E501] line
too long (91 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:275>
./pgadmin/tools/backup/tests/test_backup_message.py:52: [E501] line
too long (94 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:276>
./pgadmin/tools/backup/tests/test_backup_message.py:53: [E501] line
too long (108 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:277>
./pgadmin/tools/backup/tests/test_backup_message.py:81: [E501] line
too long (113 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:278>
./pgadmin/tools/backup/tests/test_backup_message.py:82: [E501] line
too long (94 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:279>
./pgadmin/tools/backup/tests/test_backup_message.py:83: [E501] line
too long (108 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:280>
./pgadmin/tools/backup/tests/test_backup_message.py:111: [E501] line
too long (100 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:281>
./pgadmin/tools/backup/tests/test_backup_message.py:113: [E501] line
too long (94 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:282>
./pgadmin/tools/backup/tests/test_backup_message.py:114: [E501] line
too long (108 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:283>
./pgadmin/tools/backup/tests/test_backup_message.py:147: [E501] line
too long (93 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:284>
./pgadmin/tools/backup/tests/test_batch_process.py:40: [E121]
continuation line under-indented for hanging indent
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:285>
./pgadmin/tools/backup/tests/test_batch_process.py:51: [E122]
continuation line missing indentation or outdented
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:286>
./pgadmin/tools/backup/tests/test_batch_process.py:135: [E501] line
too long (80 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:287>
./pgadmin/tools/backup/tests/test_batch_process.py:137: [E501] line
too long (83 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:288>
./pgadmin/tools/backup/tests/test_batch_process.py:138: [E122]
continuation line missing indentation or outdented
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:289>
./pgadmin/tools/backup/tests/test_batch_process.py:139: [E122]
continuation line missing indentation or outdented
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:290>
./pgadmin/tools/backup/tests/test_batch_process.py:140: [E122]
continuation line missing indentation or outdented
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:291>
./pgadmin/tools/backup/tests/test_batch_process.py:191: [E501] line
too long (81 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:292>
./pgadmin/tools/backup/tests/test_batch_process.py:203: [E501] line
too long (80 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:293>
./pgadmin/tools/backup/tests/test_batch_process.py:204: [E128]
continuation line under-indented for visual indent
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:294>
./pgadmin/tools/backup/tests/test_batch_process.py:204: [E501] line
too long (94 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:295>
./pgadmin/tools/backup/tests/test_batch_process.py:205: [E128]
continuation line under-indented for visual indent
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:296>
./pgadmin/tools/backup/tests/test_batch_process.py:205: [E501] line
too long (94 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:297>
./pgadmin/tools/backup/tests/test_batch_process.py:216: [W391] blank
line at end of file
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:298>
./pgadmin/tools/backup/tests/test_create_backup_job.py:296: [E501]
line too long (97 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:299>
./pgadmin/tools/backup/tests/test_create_backup_job.py:317: [E303] too
many blank lines (2)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:300>
./pgadmin/tools/backup/tests/test_create_backup_job.py:336: [E501]
line too long (84 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:301>
./pgadmin/tools/backup/tests/test_create_backup_job.py:371: [W391]
blank line at end of file
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:302>
2 E121 continuation line under-indented for hanging indent
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:303>
5 E122 continuation line missing indentation or outdented
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:304>
2 E128 continuation line under-indented for visual indent
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:305>
2 E251 unexpected spaces around keyword / parameter equals
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:306>
1 E303 too many blank lines (2)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:307>
24 E501 line too long (91 > 79 characters)
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:308>
2 W391 blank line at end of file
<https://gpdb-dev.bosh.pivotalci.info/teams/pgadmin/pipelines/pgadmin-patch/jobs/run-linter/builds/17#L5ad0f3d8:309>
38

For the feature tests, we realized we had to update the configuration, and
we did that, but we get the following error attached. We spent some time
trying to understand the problem but we were not successful.

Codewise:
- We just found some One Letter Variables in the code...
- Looks like there is a bug report in this area of the code and we do not
have coverage for it: https://redmine.postgresql.org/issues/3232
Looks like in some of the unit tests we only have happy path tests, maybe
we should see if there are any sad paths that also need coverage.

The configuration change, maybe need to be updated. When we install
multiple versions of postgres the binaries live in
`/usr/lib/postgresql/{{db_version}}/bin`, which makes us think that this
configuration should live near the server configuration, maybe? Also to
maintain coherency on the naming maybe we should make it all lower case.
Just as an aside, you can add the gpdb configuration as well in you patch.

Thanks
Victoria & Joao

On Wed, Apr 25, 2018 at 5:20 AM Khushboo Vashi <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

> Hi,
>
> Please find the attached patch which covers test cases for the backup
> module (RM #3206).
>
> 1. Unit test cases
> 2. End to end regression test cases
> 3. Feature test cases
>
> Thanks,
> Khushboo
>