Skip to content

Conversation

@cyphar
Copy link
Contributor

@cyphar cyphar commented Dec 28, 2016

Due to pending changes in containers/image, it is necessary that we
explicitly make compression enabled in the tests which require it. The
old behaviour was based on broken semantics (that compression would
never be un-done if you're changing compression policies).

Signed-off-by: Aleksa Sarai [email protected]

Due to pending changes in containers/image, it is necessary that we
explicitly make compression enabled in the tests which require it. The
old behaviour was based on broken semantics (that compression would
never be un-done if you're changing compression policies).

Signed-off-by: Aleksa Sarai <[email protected]>
@cyphar
Copy link
Contributor Author

cyphar commented Dec 28, 2016

DO NOT MERGE. This depends on containers/image#193.


assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", "dir:fixtures/"+t.fixture, t.remote)
assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", t.remote, "dir:"+dir)
assertSkopeoSucceeds(c, "", "--tls-verify=false", "copy", t.remote, "dir:"+dir+"::compress")
Copy link
Contributor

Choose a reason for hiding this comment

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

See the discussion in containers/image#192 and containers/image#193 — this significantly weakens the test, now we don't really know whether the compression has happened on push or pull.

@rhatdan
Copy link
Member

rhatdan commented Aug 14, 2017

Is this pull request still something we are considering?

@cyphar
Copy link
Contributor Author

cyphar commented Aug 14, 2017

I think we decided it wasn't a good idea, but that means I'll have to rethink #280.

@cyphar cyphar closed this Aug 14, 2017
@cyphar cyphar deleted the compression-tests branch August 14, 2017 16:28
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants