Skip to content

Update CI + add upgrade tests - #416

Merged
gerrod3 merged 1 commit into
pulp:masterfrom
fao89:upgrade_tests
Aug 24, 2021
Merged

Update CI + add upgrade tests#416
gerrod3 merged 1 commit into
pulp:masterfrom
fao89:upgrade_tests

Conversation

@fao89

@fao89 fao89 commented Aug 20, 2021

Copy link
Copy Markdown
Member

closes #408

Comment thread template_config.yml
update_redmine: false
upgrade_range: []
upgrade_range:
- pulp_python_branch: 3.2

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@gerrod3 @dralley could you please create this branch?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Branch should be created now. Do you also need a 3.3 branch?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

no, just the 3.2, I'll be adding the pre_upgrade tests there

@fao89
fao89 force-pushed the upgrade_tests branch 2 times, most recently from fb4db18 to cf9fa40 Compare August 20, 2021 20:58
@gerrod3

gerrod3 commented Aug 23, 2021

Copy link
Copy Markdown
Contributor

@fao89 You might want to remake this PR with the latest changes on master.

@fao89
fao89 force-pushed the upgrade_tests branch 2 times, most recently from c5e6d47 to 88c0a56 Compare August 23, 2021 20:18
@fao89 fao89 mentioned this pull request Aug 23, 2021
sed -i "/require_pulp_plugins(/d" pulp_python/tests/functional/utils.py

# Running pre upgrade tests:
pytest -v -r sx --color=yes --pyargs -capture=no pulp_python.tests.upgrade.pre

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@gerrod3 here is where the upgrade tests will start to run

@gerrod3

gerrod3 commented Aug 23, 2021

Copy link
Copy Markdown
Contributor

@fao89 It seems that your upgrade tests don't account for changes in test_requirements.txt. Going from 3.2 -> 3.4 I think I added 1 or 2 requirements to run the tests.

@fao89
fao89 force-pushed the upgrade_tests branch 6 times, most recently from 190332b to cffda68 Compare August 24, 2021 19:07
@fao89
fao89 requested a review from gerrod3 August 24, 2021 19:17

@gerrod3 gerrod3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, just a couple questions before I approve.

Comment on lines +123 to +125
# Step 8
self.assertIn("/pypi/pulp_post_upgrade_test/", distribution.base_url)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If these tests only run in the CI then the base url should be https://pulp:80/pypi/pulp_post_upgrade_test/ I think. It's fine this way too.

Also, I think the publication & distribution from the pre-test aren't destroyed correct? If not then should a test be added here to ensure you can access them from the new endpoint location?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think I can use the new endpoint when syncing, I'll change that

Comment thread pulp_python/app/utils.py
Comment on lines +153 to +155
def json_to_dict(data):
"""
Converts a JSON string into a Python dictionary.

@gerrod3 gerrod3 Aug 24, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is interesting. Why is it needed? I see its use later on in another method, just safer this way?

@fao89 fao89 Aug 24, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the post-test was breaking because the data was already a dict:

127.0.0.1 [24/Aug/2021:02:26:55 +0000] "GET /pulp/content/pulp_pre_upgrade_test/pypi/shelf-reader/json HTTP/1.0" 500 231 "-" "bandersnatch/4.4.0 (cpython 3.8.6-final0, Linux x86_64) (aiohttp 3.7.4.post0)"
  File "/usr/local/lib/python3.8/site-packages/pulpcore/content/handler.py", line 212, in stream_content
    return await self._match_and_stream(path, request)
  File "/usr/local/lib/python3.8/site-packages/pulpcore/content/handler.py", line 454, in _match_and_stream
    content_handler_result = distro.content_handler(rel_path)
  File "/usr/local/lib/python3.8/site-packages/pulp_python/app/models.py", line 108, in content_handler
    json_body = python_content_to_json(self.base_path, package_content, version=version)
  File "/usr/local/lib/python3.8/site-packages/pulp_python/app/utils.py", line 127, in python_content_to_json
    full_metadata.update({"info": python_content_to_info(latest_content[0])})
  File "/usr/local/lib/python3.8/site-packages/pulp_python/app/utils.py", line 178, in python_content_to_info
    "project_urls": json.loads(content.project_urls) or None,
  File "/usr/lib64/python3.8/json/__init__.py", line 341, in loads
    raise TypeError(f'the JSON object must be str, bytes or bytearray, '
TypeError: the JSON object must be str, bytes or bytearray, not dict

https://github.com/pulp/pulp_python/runs/3406807006?check_suite_focus=true#step:12:645

@fao89
fao89 force-pushed the upgrade_tests branch 2 times, most recently from c1f33f3 to be7c27c Compare August 24, 2021 20:00
@fao89
fao89 requested a review from gerrod3 August 24, 2021 20:44

@gerrod3 gerrod3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks!

@gerrod3
gerrod3 merged commit 547bff9 into pulp:master Aug 24, 2021
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.

Add upgrade tests

2 participants