Integration tests for boolean and assertion field operations

[grjte]

Tests for boolean and assertion field operations need to be added to the integration tests.

Tests should follow the existing pattern with the following tests added for each of the boolean field operations and the assert operation:

• a test of simple success cases
• a test with applicable failure cases, if there are any (compilation and/or execution failure)
• a proptest of the expected success case (removed as it's not necessary)

The u32 bitwise tests provide an example.

Specs for the operations are in the Miden Assembly note.

The field operations which are missing tests are:

• assert
• not
• and
• or
• xor

More information can also be seen in the field_ops assembly parsers.

[aga7hokakological]

I would like to take this issue if no one else is working on this.

[grjte]

@aga7hokakological that would be great! Please feel free to go for it

[aga7hokakological]

My question is regarding the following points:

a test with applicable failure cases, if there are any (compilation and/or execution failure)

Here in below image given failure condition for all the operations which requires test. So does that failure case comes under the above point?

[grjte]

@aga7hokakological yep, exactly. For example, we'd like to have a test that validates that calling not with a value of a > 1 causes the appropriate failure, which should be an ExecutionError.

[grjte]

@aga7hokakological I just wanted to check in and see if you have any other questions about this task that I can help with. It's also fine if it turns out you don't have time to work on it - please just let us know if that's the case :)

[aga7hokakological]

Hey @grjte sorry, I was away for a while. I had figured out about the success cases but for failed cases such as for not condition failes if a > 1. How am I supposed to write the case. because it takes only binary input so no values other than 0 and 1 are allowed.

let test = build_op_test!(asm_op, &[2]); test.expect_error(TestError::ExecutionError("ValueGreaterThanOne"));

[grjte]

Hey @aga7hokakological no problem. Is the problem that it's throwing a compilation error so it never gets a chance to execute and therefore an execution error is never triggered? If that's the case, I think it's fine to just test for a TestError::AssemblyError here. If that's not the issue can you give me a bit more detail?

[aga7hokakological]

@grjte So here,

If I do ExecutionError it gives Some(false) != Some(true) and value is not binary value.

For AssemblyError it gives None and Some(right)

Here ExecutionError is required but as it is not binary value test is failing.

[grjte]

@aga7hokakological Ok, I see. I think the issue is that you're using the wrong input to yourTestError::ExecutionError. The input should be a substring that will be contained in the expected error returned by the VM. In this case, the expected error is "...NotBinaryValue...", so if you do the following it should work:

build_op_test!(asm_op, &[2]).expect_error(TestError::ExecutionError("NotBinaryValue"));

[aga7hokakological]

@grjte Thanks. Got the silly mistake. I have corrected it. For the prop-tests too I have added tests but in there do I need to consider values as u64 or bool?
I have considered bool and all test cases pass but do I need to consider Felt data type? As field operations are different than regular values.

Test for assert works.
For assert.eq what is opcode? assert_eq does not work.

[0xkanekiken]

Test for assert works.
For assert.eq what is opcode? assert_eq does not work.

The opcode for assert.eq is assert.eq. You can either refer it from here (mentioned in the issue statement) or here

[grjte]

Thanks @0xkanekiken! Yep, that's correct - in fact, the contents of the assembly note have been migrated to our docs now, so you can check out the assembly docs here: https://maticnetwork.github.io/miden/user_docs/assembly/main.html

@aga7hokakological regarding the first question about the proptests:

• I think using bool and casting to u64 as you've done would be the right way to do this.
• You don't need to consider the Felt data type, because that's handled inside the test handler. The test just takes in integers for both inputs and expected result.

You've brought up a good point though - since the only valid inputs are boolean values, proptests are probably overkill here, and we should probably stick to unit tests, since there are only 2 or 4 options to test for each of these anyway. We use proptests for all other tests which is why I had them specified in this issue, but I shouldn't have - sorry for my mistake and the extra work

[aga7hokakological]

@grjte and @0xkanekiken Thank you. Below code helped me.

You can either refer it from here (mentioned in the issue statement) or here

you can check out the assembly docs here: https://maticnetwork.github.io/miden/user_docs/assembly/main.html

PR: #210
I made mistake while committing to my branch so I didn't mentioned the PR directly in here.

[grjte]

Closed by #210