27/07/13 20:15, intrigeri wrote: > anonym wrote (26 Jul 2013 12:49:36 GMT) : >> Hi, > >> branch: test/firewall-check-tag >> ticket: Feature #5644 <https://labs.riseup.net/code/issues/5644> > > This: >> @mac = >> REXML::Document.new(@net.xml_desc).elements['network/ip/dhcp/host/'].attributes['mac'] > > in the Sniffer class, looks like broken encapsulation (see my review > of test/reorg for details + a proposal about a similar situation). > >> - @bridge_name = bridge_name >> - @mac = mac >> + @vmnet = vmnet >> + @net = @vmnet.net >> + @bridge_name = @net.bridge_name >> + @mac = >> REXML::Document.new(@net.xml_desc).elements['network/ip/dhcp/host/'].attributes['mac'] > > It's not clear to me why @vmnet and @net are added as an instance > attributes, since they are apparently not used anywhere else in the > class or by its users. Perhaps it's not worth making the class > interface & amount of maintained state larger, if all we need in > practice is a way for some methods to get the MAC address and > bridge name?
Hopefully fixed in commit 942a030, together with the previous encapsulation issue in test/reorg. >> def assert_no_leaks > > Now that this isn't a step definition anymore, but an instance method, > I don't think `puts' is the optimal way to report detailed information > before throwing an exception. How about storing the detailed text in > a local variable, in each of the `if' blocks, and then passing this > additional information to `raise'? Fixed in commit 894f782. > It seems to me 48a32dd introduces a dependency on @custom_sniffer, > while this attribute is only created by the next commit. > This breaks bisection. Ah, right. I could squash those commits when you think this branch is merge-ready. Cheers! _______________________________________________ tails-dev mailing list [email protected] https://mailman.boum.org/listinfo/tails-dev
