Extends test set for Thread signals #5123

maciejbocianski · 2017-09-15T17:45:05Z

Description

New test suite for Thread signals

Status

READY

0xc0170 · 2017-09-19T12:49:49Z

cc @bulislaw @c1728p9

c1728p9 · 2017-09-20T14:47:12Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

-#define SIGNAL_SET_VALUE    0x01
-const int SIGNALS_TO_EMIT = 100;
-const int SIGNAL_HANDLE_DELEY = 25;
+#define TEST_STACK_SIZE   384 /* 512B stack on GCC_ARM compiler cause out of memory on some 16kB RAM boards */


Is this an old comment?

c1728p9 · 2017-09-20T14:49:13Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

+
+void run_set_clear(void)
+{
+    // 1.1


What does this comment mean?

X. - indicates subtest number
X.1, X.2, ... - indicates subtest flow

comment added

bulislaw

Could you make sure the tests are small, actual unit tests. And that the description are clear and accurate please.

bulislaw · 2017-09-26T16:02:01Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

+    When call @a signal_clr with specified signals
+    Then the specified signals are cleared
+ */
+void test_set_clear(void)


Can we not have such long tests? I think having to number and subnumber the flows means it's too complicated. Could we redesign it? Maybe split it up in separate test cases and use templates.

bulislaw · 2017-09-26T16:06:36Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

-    GREENTEA_SETUP(20, "default_auto");
+/** Test signal_wait with no signals and 0 timeout
+
+    Given a thread


Can you rewrite the description to be a bit more grammatically correct please.

bulislaw · 2017-09-26T16:07:33Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

+    When wait for all signals for 1[ms] timeout
+    Then the @a signal_wait return with osEventTimeout
+ */
+void test_wait_timeout(void)


I can clearly see two test cases here (template?).

bulislaw · 2017-09-26T16:09:18Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

+
+/** Test signal_set (in no wait state) and signal_wait
+
+    Given a thread with no signal wait state


I'm not sure what that does reading the description.

bulislaw · 2017-09-26T16:10:05Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp


+    Given a thread in signal wait state


I thin that should be rewritten to closer match the test case, something like:
Given two threads A and B
When thread A waits for a signal
and thread B calls @A signal_set
Then thread A wakes up and receives correct signal

0xc0170 · 2017-09-28T12:39:26Z

@maciejbocianski please look at the latest review comments

maciejbocianski · 2017-09-28T13:58:37Z

@0xc0170 @bulislaw done

bulislaw · 2017-09-28T14:14:33Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

+}
+
+template <int32_t signals>
+void run_wiat_clear(void)


typo wiat -> wait

bulislaw · 2017-09-28T14:20:11Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

+void run_signal_wait(void)
+{
+    osEvent ev = Thread::signal_wait(signals, timeout);
+    ret_status = ev.status;


Why don't we assert correct status here? But we pass it back to main thread?

I assumed that asserting in main function will make it easier to analyze the tests

I think it works both ways, with 'where did that value came from' factor. @0xc0170 any preferences from your side?

My preference would be the failure as soon as it happens (in this scope in this case)

bulislaw · 2017-09-28T14:20:48Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

+}
+
+template <int32_t signals, uint32_t timeout>
+void run_signal_wait2(void)


can we make the names a bit more readable? eg signal_release_wait_release or something more abstract even

bulislaw · 2017-09-28T14:22:45Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

+/** Test signal_clr with no signals specified
+
+    Given two threads A and B running, B with all signals already set
+    When execute @a signal_clr(0) with no signals specified


When thread A calls -> and similar for all of the descriptions

bulislaw · 2017-09-28T14:24:05Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

+
+/** Test if any signals are set on just created thread
+
+    Given one thread running


is running or one running thread -> and something similar for other descriptions

bulislaw · 2017-09-28T14:39:12Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

-            result = true;
-            break;
-        }
+/** Test signal_wait on thread with no signals already set - signals set one by one in loop


Could the first line for all test document, on a high level, what the tests asserts? Something like "Validate that set signals accumulate and cause signal wait to return"

bulislaw · 2017-09-28T14:41:56Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

+    Case("Test signal_wait with 1[ms] timeout and all signals set", test_wait_timeout<ALL_SIGNALS, 1>),
+    Case("Test signal_wait on thread with all signals already set", test_wait_all_already_set),
+    Case("Test signal_wait on thread with no signals already set", test_wait_all),
+    Case("Test signal_wait on thread with no signals already set - signals set one by one in loop", test_wait_all_loop)


Maybe we could also validate that:

we can set a signal then clear or wait and then set it again

what happens when we set the same signal twice

bulislaw · 2017-09-28T14:44:41Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

+    sem_a.release();
+}
+
+void run_loop_wiat_clear(void)


wiat -> wait

bulislaw · 2017-09-28T14:44:50Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

+}
+
+template <int32_t signals1, int32_t signals2>
+void run_double_wiat_clear(void)


wiat -> wait

bulislaw · 2017-09-28T14:45:36Z

TESTS/mbedmicro-rtos-mbed/signals/main.cpp

+    When execute @a signal_clr(0) with no signals specified
+    Then thread B @a signal_clr status should be ALL_SIGNALS indicating that thread B state is unchanged
+ */
+void test_clear_0(void)


I don't like 0 in names, can we have some more readable names please

theotherjimmy · 2017-10-05T15:54:54Z

@bulislaw Could you review again?

mbed-ci · 2017-10-16T02:03:28Z

Build : SUCCESS

Build number : 178
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5123/

Skipping test trigger, missing label 'NEED CI'

0xc0170 · 2017-10-19T15:33:46Z

/morph test

0xc0170 · 2017-10-19T15:34:02Z

/morph uvisor-test

mbed-ci · 2017-10-19T18:32:17Z

Test : FAILURE

Build number : 124
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5123/124

adbridge · 2017-10-24T15:17:46Z

Failing test:
tests-mbedmicro-rtos-mbed-signals Validate that call signal_set with prohibited signal doesn't change thread signals

bulislaw · 2017-10-27T15:34:17Z

/morph build

mbed-ci · 2017-10-27T16:04:36Z

Build : SUCCESS

Build number : 357
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5123/

Triggering tests

/morph test
/morph uvisor-test

mbed-ci · 2017-10-27T20:25:18Z

Test : FAILURE

Build number : 170
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5123/170

maciejbocianski · 2017-10-30T11:42:39Z

@mprse could you look on lptimer fail in test tests-mbed_drivers-lp_timer

>>> Running case #2: 'Test: LowPowerTimer - measure time accumulation.'...
[CONN][RXD] :184::FAIL: Values Not Within Delta 1 Expected 1060 Was 1058

maciejbocianski · 2017-11-03T13:19:55Z

could we rerun morph ?

mprse · 2017-11-03T13:23:40Z

@mprse could you look on lptimer fail in test tests-mbed_drivers-lp_timer

Patch for ticker test can be found here: #5403.

0xc0170 · 2017-11-13T08:52:56Z

/morph build

mbed-ci · 2017-11-13T09:25:20Z

Build : SUCCESS

Build number : 497
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5123/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

mbed-ci · 2017-11-13T10:51:46Z

Exporter Build : SUCCESS

Build number : 118
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/5123/

studavekar · 2017-11-13T16:27:57Z

/morph test

mbed-ci · 2017-11-13T18:25:07Z

Test : SUCCESS

Build number : 316
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5123/316

maciejbocianski · 2017-11-17T12:11:09Z

Patch for ticker test can be found here: #5403.

#5403 was merged, can proceed

0xc0170 · 2017-11-17T12:29:37Z

Lets retest

/morph build

mbed-ci · 2017-11-17T13:14:59Z

Build : SUCCESS

Build number : 545
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/5123/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build

mbed-ci · 2017-11-17T15:23:21Z

Test : SUCCESS

Build number : 355
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/5123/355

mbed-ci · 2017-11-17T15:26:48Z

Exporter Build : SUCCESS

Build number : 161
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/5123/

maciejbocianski force-pushed the signal_tests branch from 41513a4 to c10f8b2 Compare September 18, 2017 09:17

theotherjimmy added the needs: review label Sep 18, 2017

0xc0170 approved these changes Sep 19, 2017

View reviewed changes

c1728p9 reviewed Sep 20, 2017

View reviewed changes

maciejbocianski force-pushed the signal_tests branch from c10f8b2 to 4797a06 Compare September 22, 2017 07:48

bulislaw suggested changes Sep 26, 2017

View reviewed changes

0xc0170 added needs: work and removed needs: review labels Sep 28, 2017

maciejbocianski force-pushed the signal_tests branch from 4797a06 to cbc00dc Compare September 28, 2017 13:51

maciejbocianski force-pushed the signal_tests branch from cbc00dc to 5fd2390 Compare September 28, 2017 14:18

bulislaw suggested changes Sep 28, 2017

View reviewed changes

maciejbocianski force-pushed the signal_tests branch 2 times, most recently from de8f98d to 1b3b1ce Compare October 2, 2017 12:45

theotherjimmy added needs: review and removed needs: work labels Oct 5, 2017

bulislaw approved these changes Oct 10, 2017

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Oct 19, 2017

maciejbocianski force-pushed the signal_tests branch 2 times, most recently from 27d73ae to 37e6eee Compare October 24, 2017 13:22

maciejbocianski force-pushed the signal_tests branch from 37e6eee to e174bd4 Compare October 26, 2017 12:13

0xc0170 added needs: CI and removed needs: work labels Oct 26, 2017

adbridge added needs: work and removed needs: CI labels Oct 31, 2017

theotherjimmy added the needs: preceding PR label Nov 13, 2017

0xc0170 added needs: CI and removed needs: preceding PR needs: work labels Nov 17, 2017

0xc0170 added ready for merge and removed needs: CI labels Nov 20, 2017

theotherjimmy merged commit 593eab2 into ARMmbed:master Nov 20, 2017

theotherjimmy added release-version: 5.6.6 and removed ready for merge labels Nov 20, 2017


		/** Test signal_set (in no wait state) and signal_wait

		Given a thread with no signal wait state


		/** Test if any signals are set on just created thread

		Given one thread running

Extends test set for Thread signals #5123

Extends test set for Thread signals #5123

Uh oh!

Conversation

maciejbocianski commented Sep 15, 2017

Description

Status

Uh oh!

0xc0170 commented Sep 19, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bulislaw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Sep 28, 2017

Uh oh!

maciejbocianski commented Sep 28, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theotherjimmy commented Oct 5, 2017

Uh oh!

mbed-ci commented Oct 16, 2017

Uh oh!

0xc0170 commented Oct 19, 2017

Uh oh!

0xc0170 commented Oct 19, 2017

Uh oh!

mbed-ci commented Oct 19, 2017

Uh oh!

adbridge commented Oct 24, 2017

Uh oh!

bulislaw commented Oct 27, 2017

Uh oh!

mbed-ci commented Oct 27, 2017

Triggering tests

Uh oh!

mbed-ci commented Oct 27, 2017

Uh oh!

maciejbocianski commented Nov 17, 2017 •

edited

Loading