Fix unwanted exceptions from dac 15 class. #324
Conversation
| @@ -1430,7 +1430,7 @@ def make_jump_table_entry(cls, name, arg): | |||
| :param str name: one of: END, NOP, IDLE, CYCLE, JUMP, CHECK, RAMP | |||
| :param list[int] arg: arguments for the op. | |||
| :return: list of jump table entries | |||
| :rtype: list[jump_table.JumpEntry] | |||
| :rtype: jump_table.JumpEntry | |||
| ('NOP', [16]), | ||
| ('NOP', [20]), | ||
| ('END', [400]) | ||
| ] |
There was a problem hiding this comment.
I think it's best here to split these cases into separate test methods and give them descriptive names rather than having multiple cases within one methods with comments to explain what's different. I also generally think that if test methods are small, test one thing, and named descriptively, then docstrings are usually not needed on each method.
def test_nop_nop_from_addr_too_close(self):
entries = self.make_entries([
('NOP', [16]),
('NOP', [20]),
('END', [400])
])
with pytest.raises(AssertionError):
self.dac.make_jump_table(entries)
def test_jump_nop_from_addr_too_close(self):
...Obviously this slightly increases the amount of boilerplate, but I think it makes debugging test failures much easier.
|
A couple of comments, but otherwise LGTM. |
JulianSKelly
force-pushed
the
u/julian/test-dac
branch
from
b959122 to
49837c7
Compare
|
Addressed your comments. Will merge after I can test on hardware to make sure nothing funny happened. |
JulianSKelly
force-pushed
the
u/julian/test-dac
branch
from
49837c7 to
870a9a5
Compare
|
@maffoo Checked this on hardware and found a missing error for jump tables that are too long. Can you inspect latest commit and LGTM if ready? |
|
@maffoo please see my changes. |
|
Also looks like teamcity is barfing on futures |
|
@maffoo, can you take another look? |
|
LGTM |
Improve documentation for jump_table in dac.py Add test_dac Change dac build 15 offsets: *JT_MIN_FROM_ADDR_SPACING -> JT_MIN_CLK_CYCLES_BETWEEN_OPCODES *JT_MIN_CLK_CYCLES_BETWEEN_OPCODES: 2->4 in line with GHZDAC8 specs *JT_MIN_FROM_ADDR remove offset *JT_MIN_END_ADDR remove offset Fixes #323 *Add test, error for long jump tables Review: @maffoo
JulianSKelly
force-pushed
the
u/julian/test-dac
branch
from
d926ae4 to
2310d0e
Compare
Fixes #323
Change error checking for when two jump table commands are executed too closely in time, rather than too closely in memory.