Merge lp:~bjornt/landscape-client/apt-facade-arch into lp:~landscape/landscape-client/trunk

[Björn Tillenius]

  * Implement setting and getting the architecture in AptFacade.
  * Implement ensure_channels_reloaded() in AptFacade

[Alberto Donato (ack)]

Looks good! +1

#1:
+ first called. If it called more time, it has no effect.

typo, should be something like "If called more times"

review: Approve

[Free Ekanayaka (free.ekanayaka)]

Looks good! +1

[1]

+ apt_pkg.config.set("APT::Architecture", "i386")

I'd reccomend to not inspect the internal state of apt_pkg in the tests, as it makes them a bit "whiteboxy". Instead, I think a simple

def test_set_and_get_arch(self):
    self.facade.set_arch("some-arch")
    self.assertEqual("some-arch", self.facade.get_arch())

combined with the existing test_set_arch_get_packages is enough to fully test the behavior.

review: Approve

[Björn Tillenius (bjornt)]

On Fri, Sep 30, 2011 at 03:23:35PM -0000, Alberto Donato wrote:
> Review: Approve
>
> Looks good! +1
>
> #1:
> + first called. If it called more time, it has no effect.
>
> typo, should be something like "If called more times"

Actually, I meant "If it's called...". Fixed.

--
Björn Tillenius | https://launchpad.net/~bjornt

[Björn Tillenius (bjornt)]

On Mon, Oct 03, 2011 at 08:42:16AM -0000, Free Ekanayaka wrote:
> Review: Approve
>
> Looks good! +1
>
> [1]
>
> + apt_pkg.config.set("APT::Architecture", "i386")
>
> I'd reccomend to not inspect the internal state of apt_pkg in the tests, as it makes them a bit "whiteboxy". Instead, I think a simple
>
> def test_set_and_get_arch(self):
> self.facade.set_arch("some-arch")
> self.assertEqual("some-arch", self.facade.get_arch())
>
> combined with the existing test_set_arch_get_packages is enough to
> fully test the behavior.

Sure.

--
Björn Tillenius | https://launchpad.net/~bjornt

[lp:~bjornt/landscape-client/apt-facade-arch]

402. By Björn Tillenius on 2011-10-03

Typo.

403. By Björn Tillenius on 2011-10-03

Don't use apt internals in the tests.