[Libre-soc-isa] [Bug 686] create Power ISA test API

[bugzilla-daemon at libre-soc.org]

https://bugs.libre-soc.org/show_bug.cgi?id=686

--- Comment #48 from Luke Kenneth Casson Leighton <lkcl at lkcl.net> ---
(In reply to klehman9 from comment #47)
> https://git.libre-soc.org/?p=openpower-isa.git;a=commit;
> h=768e5265a89d4d195549b1e14dd4c033b3403210
> 
> Changed over first couple of tests in test_caller_shift_rot to use expected
> state.

oh, nice! yeah i like the idea of declaring a "blank" ExpectedState,
then modifying it afterwards, explicitly.

couple of suggestions:

1.

+    def check_regs(self, sim, e):
+        # int regs
+        for i in range(32):
+            self.assertEqual(sim.gpr(i), SelectableInt(e.intregs[i], 64),
+                "int reg %d -> sim not equal to expected." % (i))

this can be entirely replaced by.... ta-daaa.... the call to
TestState() check_regs()!

it is, after all, if you look closely, exactly the same code.

2.

name the args (use kwargs)

    e = ExpectedState(pc=4)

and put defaults of 0 for anything integers (pc) and None
for lists.

then, in the constructor, do this:

    if int_regs is None:
       int_regs = 32
    if isinstance(int_regs, int):
       int_regs = [0] * int_regs

what that does is allow us to:

a) pass in a list of expected regs
b) pass in the number of regs to be zero'd
c) default to 32 zero'd regs.

i assume you know NOT to put lists (any mutable
object) as a default parameter to a function!
because doing so is a SINGLETON pattern, believe
it or not. this results in really hard to find bugs.

-- 
You are receiving this mail because:
You are on the CC list for the bug.