Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expand test cases for artifacts creation. #966

Closed
wants to merge 1 commit into from

Conversation

nixocio
Copy link

@nixocio nixocio commented Apr 23, 2018

Add two extra test cases:

  • Create an artifact providing the wrong digest and size.
  • Create an artifact providing the right digest and size.

These tests were created using the MVP, and the auto generated API
documentation as technical reference.

See:#726

Add two extra test cases:

 * Create an artifact providing the wrong digest and size.
 * Create an artifact providing the right digest and size.

These tests were created using the MVP, and the auto generated API
documentation as technical reference.

See:pulp#726
"""Create an artifact providing the right digest and size."""
data = {'sha256': self.sha256,
'size': len(self.file['file'])}
artifact = self.client.post(ARTIFACTS_PATH, data=data, files=self.file)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new artifact is never cleaned up. One can verify this by applying the following diff:

diff --git a/pulp_smash/tests/pulp3/pulpcore/api_v3/test_crd_artifacts.py b/pulp_smash/tests/pulp3/pulpcore/api_v3/test_crd_artifacts.py
index 92d1c46..0544e1b 100644
--- a/pulp_smash/tests/pulp3/pulpcore/api_v3/test_crd_artifacts.py
+++ b/pulp_smash/tests/pulp3/pulpcore/api_v3/test_crd_artifacts.py
@@ -2,6 +2,7 @@
 """Tests that perform actions over artifacts."""
 import hashlib
 import unittest
+from pprint import pprint
 from random import randint
 
 from requests.exceptions import HTTPError
@@ -28,6 +29,9 @@ class ArtifactTestCase(unittest.TestCase, utils.SmokeTest):
         cls.file = {'file': utils.http_get(FILE2_URL)}
         cls.sha256 = hashlib.sha256(cls.file['file']).hexdigest()
 
+    def tearDown(self):
+        pprint(self.client.get(ARTIFACTS_PATH))
+
     def test_01_create(self):
         """Create an artifact by uploading a file.
 

If test_01_create_with_match_data is removed, then the print statements don't show any artifacts being left behind.

@@ -51,6 +54,23 @@ def test_01_create(self):
hashlib.sha256(files['file']).hexdigest()
)

def test_01_create_no_match_data(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I read this method name, I get the sense that no data is being passed in. This method does pass in data, though. I think method naming could be improved. Something like this might be better:

test_01_create_no_data
test_01_create_valid_data
test_01_create_invalid_data

Copy link
Contributor

@Ichimonji10 Ichimonji10 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like how different artifact creation techniques are being tested. But I think there's some way to structure this test so that artifacts are cleaned up, and so that the different techniques are each more fully tested. I'll hack at this for a bit and see what happens.

@Ichimonji10
Copy link
Contributor

What do you think about master...Ichimonji10:pr-966 ?

@nixocio
Copy link
Author

nixocio commented Apr 24, 2018

@Ichimonji10, looking right now.

@nixocio
Copy link
Author

nixocio commented Apr 24, 2018

I like the combination of data that your code introduced. Let's use this piece of code. Thank you.

@nixocio
Copy link
Author

nixocio commented Apr 24, 2018

Closed with follow-up commit 13aa943

@nixocio nixocio closed this Apr 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants