What is the impact of removing the testrunner bzr_imports_root_url? Does this cause it to default to a more suitable value?
Instead of remote_bzr_dir.transport.clone('..'), you should be able to use remote_bzr_dir.root_transport.
Some of the test changes look like drive-bys, e.g. not wanting it to leave directories behind. Is that right?
Is there a reason to join config.launchpad.bzr_imports_root_url and the branch id with path.join instead of urlutils.join?
Did you consider using more specific name than _is_import? e.g. _vfs_initial_clone ? Or what about providing an initial_clone method on the BranchPolicy itself?
What about using sets to represent files_before and files_after? That's closer to the abstract representation, because order isn't important, and entries can only appear once. I don't really understand the purpose of this loop, either.
It might be nice to turn _runWithTransformFallbackLocationHookInstalled into a ContextManager, but that's not necessary for this branch.
What is the impact of removing the testrunner bzr_imports_ root_url? Does this cause it to default to a more suitable value?
Instead of remote_ bzr_dir. transport. clone(' ..'), you should be able to use remote_ bzr_dir. root_transport.
Some of the test changes look like drive-bys, e.g. not wanting it to leave directories behind. Is that right?
Is there a reason to join config. launchpad. bzr_imports_ root_url and the branch id with path.join instead of urlutils.join?
Did you consider using more specific name than _is_import? e.g. _vfs_initial_clone ? Or what about providing an initial_clone method on the BranchPolicy itself?
What about using sets to represent files_before and files_after? That's closer to the abstract representation, because order isn't important, and entries can only appear once. I don't really understand the purpose of this loop, either.
It might be nice to turn _runWithTransfo rmFallbackLocat ionHookInstalle d into a ContextManager, but that's not necessary for this branch.