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

Fix leaking buckets in integration tests #829

Conversation

mlesniewski-reef
Copy link
Contributor

This PR fixes bucket clean ups during integration test teardown. This is achieved by tracking created and deleted buckets (both via the API and the CLI) and deleting everything which might have not been deleted automatically after each test case.

Additionally, a create_test_bucket fixture factory has been introduced, which allows for easy bucket creation. This is useful especially in tests, which need to create more than one bucket. The bucket_name fixture is still available and in use in simpler test cases.

Some buckets are still created using the create-bucket CLI command -- this was left this way on purpose, to ensure the command and its switches are covered by tests too.

The PR also re-enables parallel integration test execution (pytest-xdist) as it should work correctly now. It also introduces a change in the CI workflow config to only allow running one workflow at a time.

This should be reviewed together with corresponding changes in the SDK: Backblaze/b2-sdk-python#355

@mlesniewski-reef mlesniewski-reef force-pushed the parallel-integration-tests-fix branch from 7eb3ba7 to fa216a8 Compare September 21, 2022 07:06
Comment on lines 240 to 246
@property
def success(self):
return self.status == 0

@property
def failure(self):
return not self.success
Copy link
Collaborator

Choose a reason for hiding this comment

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

is_success
is_failure, annotate type to boolean

@@ -248,11 +231,30 @@ def read_from(self, f):
self.string = str(e)


@dataclasses.dataclass
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be frozen

Copy link
Collaborator

Choose a reason for hiding this comment

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

also please add a note in the comment to add slots=True when dropping 3.9

Copy link
Collaborator

Choose a reason for hiding this comment

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

Moved it to a NamedTuple, it's both like slots and frozen.


def run(self, args, additional_env: Optional[dict] = None) -> CommandResult:
if args:
if args[0] == 'create-bucket':
Copy link
Collaborator

Choose a reason for hiding this comment

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

we support create_bucket too, legacy syntax. Replace _ with - in args[0] before comparison

if args:
if args[0] == 'create-bucket':
raise ValueError(f'use {type(self).__name__}.create_bucket instead')
elif args[0] == 'delete-bucket':
Copy link
Collaborator

Choose a reason for hiding this comment

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

here too

@mlesniewski-reef mlesniewski-reef force-pushed the parallel-integration-tests-fix branch from fa216a8 to 8f4f431 Compare November 21, 2022 17:09
* master: (54 commits)
  Changelog adjustments
  Changelog updated
  Removed wrongly merged pyinstaller options
  PR fixes
  PR fixes - comment clarification
  Fix for tests in Python 3.7
  Linter fix
  Added a simple test to check whether incrementalMode doesn't break anything
  PR fixes
  Linter fix
  Fix for unit-tests after 1.19.0 release of b2sdk
  Ensuring that changelog is properly added to the release
  Simplified digest, using name property of a hashlib structure
  Cleaner iteration over created hash structures
  Removed limitation on file of the files from which the digest is calculated
  Ensuring that CI-only B2 installation also is silent on CI
  Changes to the test case (supporting old and new reponse) reverted
  Changelog updated
  Providing system-under-test executable from bundle to CI
  Hashes of releases added
  ...
Also limited number of parallel tests in CI
- Replaced dataclass with namedtuple
- Fixed key restrictions test with rm
@mjurbanski-reef
Copy link
Contributor

Replaced by improvements made in #946

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.

4 participants