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

Use urljoin in pulp_smash.utils #59

Merged
merged 1 commit into from
Dec 21, 2015
Merged

Use urljoin in pulp_smash.utils #59

merged 1 commit into from
Dec 21, 2015

Conversation

Ichimonji10
Copy link
Contributor

Fix #58:

Many places in the tests combine URLs and paths with simple string addition.
This has the potential to produce incorrect results. […] Pulp should produce
paths that behave correctly with urljoin, and we've had problems in the past
with developers making "relative paths" that have a leading slash.

Using urljoin instead of plain string concatenation lets Pulp Smash test the
paths returned by Pulp. If Pulp returns a relative path with a leading slash,
plain string concatenation will produce a path containing a (typically harmless
but incorrect) //, but urljoin will produce a just-plain-wrong URL.

Test results both before and after change:

============  =========================================
Pulp Version  Test Suite Results
============  =========================================
2.7           OK (skipped=13)
dev (2.8)     FAILED (failures=1, errors=2, skipped=12)
============  =========================================

Command used:

PULP_SMASH_CONFIG_FILE=$PS_CONFIG \
python -m unittest2 discover pulp_smash.tests

@Ichimonji10
Copy link
Contributor Author

@mhrivnak

Fix #58:

> Many places in the tests combine URLs and paths with simple string addition.
> This has the potential to produce incorrect results. […] Pulp should produce
> paths that behave correctly with urljoin, and we've had problems in the past
> with developers making "relative paths" that have a leading slash.

Using urljoin instead of plain string concatenation lets Pulp Smash test the
paths returned by Pulp. If Pulp returns a relative path with a leading slash,
plain string concatenation will produce a path containing a (typically harmless
but incorrect) `//`, but urljoin will produce a just-plain-wrong URL.

Test results both before and after change:

    ============  =============================
    Pulp Version  Test Suite Results
    ============  =============================
    2.7           OK (skipped=13)
    dev (2.8)     FAILED (errors=2, skipped=13)
    ============  =============================

Command used:

    PULP_SMASH_CONFIG_FILE=$PS_CONFIG \
    python -m unittest2 discover pulp_smash.tests
@Ichimonji10 Ichimonji10 merged commit 956efdf into pulp:master Dec 21, 2015
@Ichimonji10 Ichimonji10 deleted the urljoin branch December 21, 2015 17:33
@elyezer
Copy link
Contributor

elyezer commented Dec 21, 2015

👍

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