Skip to content

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.

# 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().


# 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