Add basic tests for repository CRUD. #28

jeremycline · 2015-11-05T15:23:11Z

This commit also introduces a utils module which currently only
provides a function to generate random strings using uuid.uuid4.

Ichimonji10 · 2015-11-06T16:19:49Z

pulp_smash/tests/platform/api_v2/test_repository.py

@@ -0,0 +1,226 @@
+# coding=utf-8
+"""Test the `repository`_ API endpoints."""


No target is given for the "repository" reference. As a result, it should be broken. You can provide a target like so:

.. repository: https://example.com

Of course, you don't actually want to link the user to example.com.

Ichimonji10 · 2015-11-06T18:53:11Z

Can you run the new tests and paste the output here when done?

jeremycline · 2015-11-09T15:55:10Z

Output is:

(pulp-smash)[vagrant@dev pulp-smash]$ python -m unittest2 discover pulp_smash
....F.........................
======================================================================
FAIL: test_body (tests.platform.api_v2.test_login.LoginFailureTestCase)
Assert that the response is valid JSON and has correct keys.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vagrant/devel/pulp-smash/pulp_smash/tests/platform/api_v2/test_login.py", line 69, in test_body
    _LOGIN_ERROR_KEYS,
AssertionError: Items in the first set but not the second:
u'http_request_method'
u'auth_error_code'
Items in the second set but not the first:
u'href'

======================================================================
FAIL: test_status_code (tests.platform.api_v2.test_repository.CreateFailureTestCase) [(400, [u'Incorrect data type'])]
Assert that each response has the expected HTTP status code.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vagrant/devel/pulp-smash/pulp_smash/tests/platform/api_v2/test_repository.py", line 118, in test_status_code
    self.assertEqual(response.status_code, self.bodies[i][0])
AssertionError: 500 != 400

======================================================================
FAIL: test_use_deleted_user (tests.platform.api_v2.test_user.ReadUpdateDeleteTestCase) [put]
Assert that one cannot read, update or delete a deleted user.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/vagrant/devel/pulp-smash/pulp_smash/tests/platform/api_v2/test_user.py", line 169, in test_use_deleted_user
    self.assertEqual(response.status_code, 404)
AssertionError: 400 != 404

----------------------------------------------------------------------
Ran 32 tests in 2.966s

FAILED (failures=3)

I'm not sure what the problem is with the user tests, but the repository test should be failing (https://pulp.plan.io/issues/1356).

asmacdo · 2015-11-09T17:53:08Z

Line 169 is this failure: #34

Ichimonji10 · 2015-11-09T17:57:07Z

I believe the failing user test is an expected failure. There's a known bug tested by that API call, and... I don't have a link to it.

Ichimonji10 · 2015-11-09T17:59:29Z

pulp_smash/tests/platform/api_v2/test_repository.py

+            (400, {'id': None}),
+            (400, ['Incorrect data type']),
+            (400, {'missing_required_keys': 'id'}),
+            (409, {'id': identical_id}),


As per Exception Handling, these four API calls should return a call report with ERROR_KEYS. Can we add in a test method for that?

Ichimonji10 · 2015-11-09T18:01:33Z

The two new .rst files which have been added to the documentation are not accessible. Can you link to them from docs/api.rst?

Ichimonji10 · 2015-11-09T18:02:46Z

pulp_smash/tests/platform/api_v2/test_repository.py

+        attributes = {key: attributes[key] for key in self.bodies[0].keys()}
+        self.assertEqual(self.bodies[0], attributes)
+
+    def test_update_attributes_spawned_tasks(self):


Can you add # noqa pylint:disable=invalid-name to the end of this line?

Ichimonji10 · 2015-11-09T18:10:46Z

As per #28 (comment), I'm going to refactor this module at a later point. Can you add # pylint:disable=duplicate-code to the top of the module of tests, with a comment explaining what's going on?

Ichimonji10 · 2015-11-09T18:11:10Z

I think that's about it. If you can address the points above, I think this is great.

asmacdo · 2015-11-09T18:14:14Z

pulp_smash/tests/platform/api_v2/test_repository.py

+            with self.subTest(key):
+                self.assertIn(key, response['result'])
+                self.assertEqual(
+                    self.update_body[key],


self.update_body['delta'][key]

Nice catch. This could also be written as:

for key, value in self.update_body['delta'].items(): with self.subTest(key=key): self.assertIn(key, response['result']) self.assertEqual(value, response['result'][key])

Ichimonji10 · 2015-11-10T18:55:16Z

@jeremycline Please rebase and apply this patch:

diff --git a/Makefile b/Makefile
index 22b7274..1dada96 100644
--- a/Makefile
+++ b/Makefile
@@ -19,7 +19,9 @@ docs-clean:

 lint:
    flake8 .
-   pylint --reports=n --disable=I docs/conf.py pulp_smash tests setup.py
+   pylint --reports=n --disable=I docs/conf.py tests setup.py \
+       pulp_smash/{__init__,__main__,config,constants,utils}.py
+   pylint --reports=n --disable=I,duplicate-code pulp_smash/tests/

 test:
    python $(TEST_OPTIONS)

The above patch causes pylint to ignore any duplicate code in the pulp_smash.tests module. I dislike just ignoring this warning - it's there for a reason, and the proper solution is to use composition or some other mechanism to pull the commonalities out. But I'm willing to take on that technical debt for now.

jeremycline · 2015-11-10T20:53:32Z

@Ichimonji10 I have made it so. Note that I had to add SHELL=/bin/bash in the Makefile to support the bash path expansion in that patch.

Ichimonji10 · 2015-11-10T20:56:32Z

@jeremycline Will that cause the makefile to break on systems that may not have bash installed, such as Mac OS (or Cygwin, or other more exotic *nix-like distros)?

jeremycline · 2015-11-10T20:58:31Z

@Ichimonji10 Yes. Does /usr/bin/env sh work in OSX/*nix?

Ichimonji10 · 2015-11-10T21:01:40Z

I don't know. We can just expand those braces:

diff --git a/Makefile b/Makefile
index 22b7274..1dada96 100644
--- a/Makefile
+++ b/Makefile
@@ -19,7 +19,9 @@ docs-clean:

 lint:
    flake8 .
-   pylint --reports=n --disable=I docs/conf.py pulp_smash tests setup.py
+   pylint --reports=n --disable=I docs/conf.py tests setup.py \
+       pulp_smash/__init__.py \
+       pulp_smash/__main__.py \
+       pulp_smash/config.py \
+       pulp_smash/constants.py \
+       pulp_smash/utils.py
+   pylint --reports=n --disable=I,duplicate-code pulp_smash/tests/

 test:
    python $(TEST_OPTIONS)

This commit also introduces a ``utils`` module which currently only provides a function to generate random strings using ``uuid.uuid4``.

jeremycline · 2015-11-10T21:06:50Z

I guess we could just do that.

Ichimonji10 · 2015-11-10T21:07:32Z

Ugly, but highly compatible.

jeremycline · 2015-11-10T21:08:28Z

Very true. It is done.

Ichimonji10 · 2015-11-10T21:13:55Z

jeremycline force-pushed the test-repository-basics branch from 04d619e to 1e6fff0 Compare November 5, 2015 15:35

Ichimonji10 reviewed Nov 6, 2015
View reviewed changes

jeremycline force-pushed the test-repository-basics branch from 1e6fff0 to f8f863e Compare November 9, 2015 15:43

jeremycline mentioned this pull request Nov 9, 2015

Update make test-coverage make target #35

Closed

Ichimonji10 reviewed Nov 9, 2015
View reviewed changes

asmacdo reviewed Nov 9, 2015
View reviewed changes

jeremycline force-pushed the test-repository-basics branch from f8f863e to a150499 Compare November 9, 2015 18:54

Ichimonji10 mentioned this pull request Nov 9, 2015

Refactor user and repository CRUD tests #37

Closed

jeremycline force-pushed the test-repository-basics branch 2 times, most recently from 4222c79 to bd4cc63 Compare November 10, 2015 20:49

Add basic tests for repository CRUD.

cda18c4

This commit also introduces a ``utils`` module which currently only provides a function to generate random strings using ``uuid.uuid4``.

jeremycline force-pushed the test-repository-basics branch from bd4cc63 to cda18c4 Compare November 10, 2015 21:06

Ichimonji10 merged commit cda18c4 into pulp:master Nov 12, 2015

jeremycline deleted the test-repository-basics branch November 12, 2015 15:08

Ichimonji10 mentioned this pull request Nov 12, 2015

Adding api for repositories and tasks and few repository tests #20

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add basic tests for repository CRUD. #28

Add basic tests for repository CRUD. #28

jeremycline commented Nov 5, 2015

Ichimonji10 Nov 6, 2015

Ichimonji10 commented Nov 6, 2015

jeremycline commented Nov 9, 2015

asmacdo commented Nov 9, 2015

Ichimonji10 commented Nov 9, 2015

Ichimonji10 Nov 9, 2015

Ichimonji10 commented Nov 9, 2015

Ichimonji10 Nov 9, 2015

Ichimonji10 commented Nov 9, 2015

Ichimonji10 commented Nov 9, 2015

asmacdo Nov 9, 2015

Ichimonji10 Nov 9, 2015

Ichimonji10 commented Nov 10, 2015

jeremycline commented Nov 10, 2015

Ichimonji10 commented Nov 10, 2015

jeremycline commented Nov 10, 2015

Ichimonji10 commented Nov 10, 2015

jeremycline commented Nov 10, 2015

Ichimonji10 commented Nov 10, 2015

jeremycline commented Nov 10, 2015

Ichimonji10 commented Nov 10, 2015

		@@ -0,0 +1,226 @@
		# coding=utf-8
		"""Test the `repository`_ API endpoints."""

Add basic tests for repository CRUD. #28

Add basic tests for repository CRUD. #28

Conversation

jeremycline commented Nov 5, 2015

Ichimonji10 Nov 6, 2015

Choose a reason for hiding this comment

Ichimonji10 commented Nov 6, 2015

jeremycline commented Nov 9, 2015

asmacdo commented Nov 9, 2015

Ichimonji10 commented Nov 9, 2015

Ichimonji10 Nov 9, 2015

Choose a reason for hiding this comment

Ichimonji10 commented Nov 9, 2015

Ichimonji10 Nov 9, 2015

Choose a reason for hiding this comment

Ichimonji10 commented Nov 9, 2015

Ichimonji10 commented Nov 9, 2015

asmacdo Nov 9, 2015

Choose a reason for hiding this comment

Ichimonji10 Nov 9, 2015

Choose a reason for hiding this comment

Ichimonji10 commented Nov 10, 2015

jeremycline commented Nov 10, 2015

Ichimonji10 commented Nov 10, 2015

jeremycline commented Nov 10, 2015

Ichimonji10 commented Nov 10, 2015

jeremycline commented Nov 10, 2015

Ichimonji10 commented Nov 10, 2015

jeremycline commented Nov 10, 2015

Ichimonji10 commented Nov 10, 2015