[rb-general] Refactoring adt_testbed on reprotest

[Ximin Luo]

Santiago Torres:
> Hello everyone.
> 
> As I mentioned on the previous IRC meeting. I've been trying to make the
> reprotest suite distribution agnostic[1]. I was hoping we could spark
> some discussion of whether this is the right approach. I'm trying to
> make it so that any distro can write a module/class to abstract away
> their specific toolchain from adt_testbed (I can forward the
> formatted-patches to the list, if necessary)
> 
> I also removed some adt_testbed code[2] that I don't think would hit from
> the context of reprotest. 
> 
> Thoughts?
> 
> Thanks!
> -Santiago.
> 
> P.S. It may also be worth checking [3] regarding factoring out
> adt_testbed.
> 
> [1] https://github.com/SantiagoTorres/reprotest/commits/distro_agnostic
> [2] https://github.com/SantiagoTorres/reprotest/commit/06456d354ba5c0579cdf9ebd6e7621fac174033a
> [3] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=836520 
> 

Hey Santiago, I think this is the right approach. It looks very good, thanks for doing it! Could you tidy up the code first a bit before I merge it?

- Remove first two commits ("factor out utils" and "remove unused code") - maybe stash them somwhere else, on a different branch. They're good in principle, but we'd like to keep diffs small for the time being to make it easier to merge to autopkgtest later. They seem to be unnecessary for the subsequent commits.

- *_interface modules:
  - Lots of unused imports, these can be removed. You can run something like "pylint --reports=n reprotest/ | grep import"
  - Copyright would probably be you, not Canonical
  - You copied an old TODO comment from the old code, this can be removed
  - Classes are CamelCase in python, so SystemInterface not system_interface etc.

- When choosing which interface class to use, the importlib/get_interface stuff is not necessary. You can just create a dictionary and index into that, e.g. { "debian": DebianInterface() etc }, it takes a tiny amount of manual extra effort to update but is cleaner than using importlib.

- naming:
  - get_arch_exec -> get_arch, since you call it system_arch in the Testbed class
  - get_testbed_packages -> get_installed_packages, it is not testbed-specific

You can use "git rebase --interactive" to make the history nicer, after doing these changes.

In the meantime, I'll experiment with re-importing the current autopkgtest code and applying a minimal version of the diff that I posted to that bug report:

https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=836520;filename=reprotest-adt.diff;msg=10

If all goes well, it should be possible to merge or rebase your commits on top of this, after you're done tidying it up.

X

-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git