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

Testing: Add disposition with SIP to fixture. #5849

Merged
merged 1 commit into from
Aug 5, 2019
Merged

Conversation

lukasgraf
Copy link
Contributor

This adds a disposition to the fixture which is in state disposition-state-disposed and has a SIP package, ready to be delivered.

This is necessary because transports (which will be implemented as part of #5167) need to access the underlying blob on the filesystem. The blob will only have a stable filesystem location once it's committed though. Because in our IntegrationTestCase we only emulate transaction.commit(), but never actually do a hard commit, this requires the blob to have already been created as part of the fixture to be actually committed and accessible.

Checkliste

  • Aktualisierung Dokumentation vorhanden/nötig?

@lukasgraf lukasgraf added this to the Sprint #3 milestone Aug 5, 2019
@lukasgraf lukasgraf requested a review from a team August 5, 2019 09:23
Copy link
Contributor

@njohner njohner left a comment

Choose a reason for hiding this comment

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

LGTM. Just a bit hard to judge whether we really need a disposition that has been disposed in the fixture or if using self.disposition and making the disposition-transition-dispose in the tests that need the SIP would be good enough would be enough.

@njohner
Copy link
Contributor

njohner commented Aug 5, 2019

Ok never mind my comment, I guess I did not properly read your PR comment... it's all clear why we need this!

@lukasgraf lukasgraf merged commit 6725397 into master Aug 5, 2019
@lukasgraf lukasgraf deleted the lg-sip-fixture branch August 5, 2019 13:28
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