Conversation
esp_idf/spotflow/device_sdk/examples/unit_test/test/main/example_unit_test.c
Outdated
Show resolved
Hide resolved
| read_queue_and_verify(log_msg); // First verify the message is logged | ||
|
|
||
| // Now check if the CBOR data has the correct key-value pairs | ||
| uint8_t* cbor_data = (uint8_t*)queue_msg.ptr; // The CBOR encoded message | ||
| size_t cbor_len = queue_msg.len; |
There was a problem hiding this comment.
read_queue_and_verify calls spotflow_queue_free(&queue_msg); that frees queue_msg.ptr but then you do uint8_t* cbor_data = (uint8_t*)queue_msg.ptr; and use cbor_data
esp_idf/spotflow/device_sdk/test/log_backend/test_log_backend.c
Outdated
Show resolved
Hide resolved
| TEST_SPOTFLOW_ASSERT_TRUE(cbor_buf != NULL); | ||
|
|
||
| // The CBOR should encode empty string for "source" | ||
| TEST_SPOTFLOW_ASSERT_TRUE(memmem(cbor_buf, cbor_len, "", 0) != NULL); |
There was a problem hiding this comment.
does this make any sense? basically searching for zero-legth string in buffer - always return pointer to buffer
| TEST_SPOTFLOW_ASSERT_TRUE(memmem(cbor_buf, cbor_len, "Hello CBOR", strlen("Hello CBOR")) != NULL); | ||
|
|
||
| // Check metadata sequence number (encoded as integer somewhere in CBOR) | ||
| TEST_SPOTFLOW_ASSERT_TRUE(memmem(cbor_buf, cbor_len, &meta.sequence_number, 1) != NULL); |
There was a problem hiding this comment.
why you are using size 1 and not the size of sequence_number
| @@ -0,0 +1 @@ | |||
| ../../../LICENSE.MD No newline at end of file | |||
There was a problem hiding this comment.
inconsistent file naming - this file is .md, referenced is .MD
There was a problem hiding this comment.
This is like for reference due to some reason the esp idf component website wasn't picking up the license.MD so used this one and it worked so didn't touch it afterwards same is for changelog as well. This was just a symbolink link created for the UI.
esp_idf/spotflow/device_sdk/examples/unit_test/test/main/example_unit_test.c
Outdated
Show resolved
Hide resolved
| struct message_metadata meta = { | ||
| .sequence_number = 1, | ||
| .uptime_ms = 1000, | ||
| .source = "unit_test" | ||
| }; |
There was a problem hiding this comment.
- why is severity in metadata it seems that in cbor serialization, the parameter from spotflow_log_cbor_send is taken and metadata's severity is ignored.
- Could you please also test that severity is serialized correctly?
There was a problem hiding this comment.
Okay this one went through the cracks. I was looking into the backend logic as well and their I missed it i kept on using the severity as a seperate argument instead of using the metadata. I will fix it.
| @@ -0,0 +1,24 @@ | |||
| # Test skeleton | |||
There was a problem hiding this comment.
Isn't it confusing to have unit_test in examples? Maybe we could have it next to the examples folder, what do you think?
There was a problem hiding this comment.
This is like a test skeleton like the actual tests are in the test folder this example just shows how to use them or for us testing. It's a copy paste of the esp idf testing example.
With changes to cmake file
So, we can move it or remove it entirely from the component and just use it internally.
|
Generally, I would prefer to test the CBOR using a deserializer - either the same one used for serialization or a different one (preferred if possible). Searching bytes in serialized cbor can lead to fragile tests. e.g., if you serialize uint64_t to CBOR and its value is smaller than 255, it will get encoded into a single byte - optimization done by cbor, so you won't be able to do the mem search. |
|
Yeah I will change this. |
3 participants
Added tests for esp idf components.