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

Accept all content types for README files already in code #176

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

lexming
Copy link
Contributor

@lexming lexming commented Dec 9, 2020

There is already code in vsc-install checking for those README file formats recommended by pypi. However, currently the only allowed README file is README.md. This PR takes the existing list in readme_content_types and uses it to check for existing README files as well (by order of preference).

The list of allowed README files becomes:

  • README.md, README.rst, README.txt, README

@stdweird
Copy link
Member

stdweird commented Dec 9, 2020

jenkins failures

======================================================================

ERROR: test_parse_target (test.shared_setup.TestSetup)

Test for parse target

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/var/lib/jenkins/jobs/hpcugent_github.com/jobs/vsc-install/branches/PR-176/workspace/test/shared_setup.py", line 217, in test_parse_target

    new_target = setup.parse_target(package)

  File "lib/vsc/install/shared_setup.py", line 1403, in parse_target

    readmetxt = _read(self.readme)

AttributeError: 'vsc_setup' object has no attribute 'readme'


======================================================================

ERROR: test_parse_target_dependencies (test.shared_setup.TestSetup)

Test injecting dependency_links in parse_target

----------------------------------------------------------------------

Traceback (most recent call last):

  File "/var/lib/jenkins/jobs/hpcugent_github.com/jobs/vsc-install/branches/PR-176/workspace/test/shared_setup.py", line 243, in test_parse_target_dependencies

    new_target = setup.parse_target(package)

  File "lib/vsc/install/shared_setup.py", line 1403, in parse_target

    readmetxt = _read(self.readme)

AttributeError: 'vsc_setup' object has no attribute 'readme'


----------------------------------------------------------------------

Ran 46 tests in 22.083s


FAILED (errors=2)

@lexming
Copy link
Contributor Author

lexming commented May 28, 2021

I'll try once more to get this merged. It is now synced with master.
@stdweird: can you tell me the new error of the tests?

@stdweird
Copy link
Member

@lexming weird it doesn't fail locally then.

======================================================================

FAIL: test_setup_cfg (test.shared_setup.TestSetup)

Test generating of setup.cfg.

----------------------------------------------------------------------

Traceback (most recent call last):

  File "lib/vsc/install/testing.py", line 76, in assertEqual

    super(TestCase, self).assertEqual(a, b)

  File "/usr/lib64/python3.6/unittest/case.py", line 846, in assertEqual

    assertion_func(first, second, msg=msg)

AssertionError: '[bdist_rpm]' != '[bdist_rpm]\n\n[metadata]\n\ndescription-file = README.md'

- [bdist_rpm]

+ [bdist_rpm]


[metadata]


description-file = README.md



During handling of the above exception, another exception occurred:


Traceback (most recent call last):

  File "/var/lib/jenkins/jobs/hpcugent_github.com/jobs/vsc-install/branches/PR-176/workspace/test/shared_setup.py", line 324, in test_setup_cfg

    self.assertEqual(read_setup_cfg(), expected)

  File "lib/vsc/install/testing.py", line 98, in assertEqual

    raise AssertionError("%s:\nDIFF%s:\n%s" % (msg, limit, ''.join(diff[:self.ASSERT_MAX_DIFF])))

AssertionError: '[bdist_rpm]' != '[bdist_rpm]\n\n[metadata]\n\ndescription-file = README.md'

- [bdist_rpm]

+ [bdist_rpm]


[metadata]


description-file = README.md

:

DIFF:

@lexming
Copy link
Contributor Author

lexming commented May 19, 2022

@stdweird I finally updated this PR and it passes the tests now! 🍾 Sorry for the long delay on this.

@@ -313,7 +313,10 @@ def read_setup_cfg():
os.chdir(self.tmpdir)

# test with minimal target
vsc_setup.build_setup_cfg_for_bdist_rpm({})
target = {
'description_file': 'README.md',
Copy link
Member

Choose a reason for hiding this comment

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

why do you need to set this. it's the default value right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only needed in this unit test with an artificial target. In a real scenario the description_file attribute will be set by the call to locate_readme() in action_target(), which looks for the actual README file in the repo directory. The only case where a target would have no description_file is if there is no README, which will error out before reaching build_setup_cfg_for_bdist_rpm().

@@ -325,6 +328,7 @@ def read_setup_cfg():

# realistic target
target = {
'description_file': 'README.md',
Copy link
Member

Choose a reason for hiding this comment

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

not sure why this is needed? we're not going to change all the setup.py to add the old default...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need to change any existing setup.py, as explained in my previous comment, this is only needed for this artificial target. From now on all targets will be generated with a description_file attribute pointing to the actual README file in the repo.

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.

2 participants