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

Restore PATH_SCHEMA to be AnyString #248

Closed

Conversation

MVrachev
Copy link
Collaborator

@MVrachev MVrachev commented Jun 29, 2020

Fixes issue #: theupdateframework/python-tuf#1059

Description of the changes being introduced by the pull request:

In one of the latest commits AnyNonemptyString was introduced
and with it, PATH_SCHEMA was changed to AnyNonemptyString from
AnyString. This led to unit tests failures in TUF.

One could argue what is the right approach: to consider
that PATH_SCHEMA is every non-empty string or every string.
I think this is not of much importance, but for TUF and
possibly for other securesystemslib adopters, this leads to tests
failures. The reason is that there are multiple functions
with arguments which should conformant against the PATH_SCHEMA
but have default values of ''

See :

Signed-off-by: Martin Vrachev [email protected]

Please verify and check that the pull request fulfils the following
requirements
:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

In one of the latest commits AnyNonemptyString was introduced
and with it, PATH_SCHEMA was changed to AnyNonemptyString from
AnyString. This led to unit tests failures in TUF.
See: theupdateframework/python-tuf#1059

One could argue what is the right approach: to consider
that PATH_SCHEMA is every non-empty string or every string.
I think this is not of much importance, but for TUF and
possibly for other securesystemslib adopters, this leads to tests
failures. The reason is that there are multiple functions
with arguments which should conformant against the PATH_SCHEMA
but have default values of ''.

See :
- "prefix" argument in load_repository function
- "location_in_repository" argument in create_new_project function
both of which are in tuf/developer_tool.py

Signed-off-by: Martin Vrachev <[email protected]>
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0005%) to 98.944% when pulling 6255ea9 on MVrachev:fix-failing-tuf-tests into 7531747 on secure-systems-lab:master.

@lukpueh
Copy link
Member

lukpueh commented Jun 30, 2020

I'd rather update TUF than restore the change here. We just added this in #244 for good reason, that is to fix #241.

@MVrachev
Copy link
Collaborator Author

Yes, you are right.
I will close that pr and submit a pr in tuf.

@MVrachev MVrachev closed this Jun 30, 2020
@MVrachev MVrachev deleted the fix-failing-tuf-tests branch June 30, 2020 16:06
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.

3 participants