Disables flash clock and cache test #6340

cmonr · 2018-03-12T23:37:42Z

Has been causing intermittent morph test failures with NRF52_DK boards

Description

Disables the flash clock and cache tests. For a few weeks, this test would intermittently fail specifically against the NRF52_DK built against the GCC_ARM compiler.

An example failure:

[1520888954.03][CONN][RXD] >>> Running case #6: 'Flash - clock and cache test'...
[1520888954.11][CONN][INF] found KV pair in stream: {{__testcase_start;Flash - clock and cache test}}, queued...
[1520888954.77][CONN][RXD] :273::FAIL: Values Not Within Delta 3016 Expected 603210 Was 590241
[1520888954.77][CONN][INF] found KV pair in stream: {{__testcase_finish;Flash - clock and cache test;0;1}}, queued...
[1520888954.86][CONN][RXD] >>> 'Flash - clock and cache test': 0 passed, 1 failed with reason 'Assertion Failed'

Note: This is not expected to be a permanent fix, but should make tests more stable and save us some test-restart pains.

Pull request type

cmonr · 2018-03-12T23:37:54Z

/morph build

cmonr · 2018-03-12T23:59:32Z

Restarting. Let other PR take priority. (#6341)

/morph build

mbed-ci · 2018-03-13T00:42:23Z

Build : SUCCESS

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

Triggering tests

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

mbed-ci · 2018-03-13T04:59:46Z

Exporter Build : ABORTED

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

studavekar · 2018-03-13T05:38:23Z

TESTS/mbed_hal/flash/functional_tests/main.cpp

-    const int acceptable_range = timer_diff_start / (ALLOWED_DRIFT_PPM);
-    TEST_ASSERT_UINT32_WITHIN(acceptable_range, timer_diff_start, timer_diff_end);
-}
+//void flash_clock_and_cache_test()


can we just comment call to this function.

I didn't want to throw an extra warning about an unused function.

0xc0170 · 2018-03-13T08:12:10Z

Are we able to reproduce this error? Disabling a test is not an answer usually.

cmonr · 2018-03-13T14:25:18Z

Are we able to reproduce this error? Disabling a test is not an answer usually.

Not realiably, but it's a failure error we've been seeing more of lately.
This PR was meant as a short term solution to the problem, with the (proper) long term solution to come after.

0xc0170 · 2018-03-13T15:03:43Z

This PR was meant as a short term solution to the problem, with the (proper) long term solution to come after.

Wouldn't be better to disable it just for one target then?

studavekar · 2018-03-13T15:43:31Z

Are we able to reproduce this error? Disabling a test is not an answer usually.

we have seen this often on NRF52840_DK, it would be a good candidate to reproduce.

cmonr · 2018-03-13T15:46:06Z

Wouldn't be better to disable it just for one target then?

Wasn't aware that was a thing. Will update the PR to do just that.

studavekar · 2018-03-13T15:48:07Z

Wouldn't be better to disable it just for one target then?
Wasn't aware that was a thing. Will update the PR to do just that.

Probably nordic targets may be?

cmonr · 2018-03-16T20:13:54Z

@studavekar Re-review requested.

Tested by compiling against NRF52_DK and K66F.

Will squash commits after an OK is given. Not completely sure this is the proper way to go about doing this.

bridadan · 2018-03-16T20:27:39Z

@cmonr I'd change your check to the following:

#if !defined(TARGET_NRF52840_DK) || !defined(TARGET_NRF52_DK)

That should disable the test just for NRF52840_DK and NRF52_DK.

bridadan · 2018-03-16T20:29:16Z

Another option is you could use those same checks to increase the expected delta for those targets.

studavekar · 2018-03-16T20:31:37Z

I was thinking we protect the test case itself, instead of a pass

Case cases[] = {
    Case("Flash - init", flash_init_test),
    Case("Flash - mapping alignment", flash_mapping_alignment_test),
    Case("Flash - erase sector", flash_erase_sector_test),
    Case("Flash - program page", flash_program_page_test),
    Case("Flash - buffer alignment test", flash_buffer_alignment_test),
#if !defined(TARGET_NRF52840_DK) || !defined(TARGET_NRF52_DK)
    Case("Flash - clock and cache test", flash_clock_and_cache_test),
#endif
};

SenRamakri · 2018-03-16T20:39:07Z

Yes, its better if we can avoid running the tests on targets where its not supported instead of providing a "False Pass" indication.

bridadan · 2018-03-16T20:43:59Z

Agreed @studavekar and @SenRamakri

cmonr · 2018-03-16T22:33:44Z

Will make changes per recommendations.

0xc0170 · 2018-03-19T10:37:53Z

Will make changes per recommendations.

@cmonr Can we resolve this today? asap

theotherjimmy

@cmonr This is not how we disable tests for a target, or series of targets.

Instead, could you add a

#ifdef NORDIC
#error [NOT SUPPORTED] ...
#endif

somewhere in this file.

theotherjimmy

I take it back. I must have had an old cached copy of the code, where all tests were disabled.

This is meant to be a temporary fix until the issue has been root caused, and Jenkins CI is no longer intermittently failing.

cmonr · 2018-03-19T17:01:22Z

Re-requesting reviews.

@studavekar @0xc0170 @bridadan @adbridge @theotherjimmy

theotherjimmy

Pretty simple.

bridadan

No problems here

studavekar · 2018-03-20T04:52:16Z

/morph build

mbed-ci · 2018-03-20T05:38:09Z

Build : SUCCESS

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

Triggering tests

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

mbed-ci · 2018-03-20T07:46:00Z

Exporter Build : SUCCESS

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

0xc0170

We shall not forget to fix this and reenable it later :)

0xc0170 · 2018-03-20T10:11:26Z

cc @marcuschangarm (to be aware of this)

mbed-ci · 2018-03-20T12:15:03Z

Test : SUCCESS

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

studavekar · 2018-03-20T16:43:57Z

@cmonr @0xc0170 @theotherjimmy should we merge this one?

cmonr · 2018-04-02T13:32:25Z

Making a note here. This PR does not successfully disable the test.

0xc0170 · 2018-04-03T12:06:28Z

Making a note here. This PR does not successfully disable the test.

Looking at the macro, would it be better TARGET_MCU_NRF52 ? would that solve it or what is the problem?

cmonr · 2018-04-03T15:54:36Z

Looking at the macro, would it be better TARGET_MCU_NRF52 ? would that solve it or what is the problem?

I think that would be a better macro. Working on it right now. Will provide pre/post test logs with new PR to confirm the test is actually disabled.

cmonr added the needs: review label Mar 12, 2018

cmonr requested review from 0xc0170, adbridge and studavekar March 12, 2018 23:37

studavekar suggested changes Mar 13, 2018

View reviewed changes

cmonr added needs: work and removed needs: review labels Mar 13, 2018

cmonr requested a review from bridadan March 16, 2018 20:10

hug-dev mentioned this pull request Mar 19, 2018

Add include search path when preprocessing assembly files for uvision and make_armc5 exports #6356

Merged

theotherjimmy suggested changes Mar 19, 2018

View reviewed changes

theotherjimmy previously approved these changes Mar 19, 2018

View reviewed changes

cmonr dismissed theotherjimmy’s stale review via f18ecd1 March 19, 2018 16:04

cmonr force-pushed the disable-failing-flash-test branch from 7f54bb7 to f18ecd1 Compare March 19, 2018 16:04

cmonr added needs: review and removed needs: work labels Mar 19, 2018

Disabled flash clock and cache test for NRF52 MCUs.

9f63013

This is meant to be a temporary fix until the issue has been root caused, and Jenkins CI is no longer intermittently failing.

studavekar mentioned this pull request Mar 19, 2018

LoRa refactoring #6279

Merged

5 tasks

cmonr force-pushed the disable-failing-flash-test branch from f18ecd1 to 9f63013 Compare March 19, 2018 17:00

theotherjimmy approved these changes Mar 19, 2018

View reviewed changes

bridadan approved these changes Mar 19, 2018

View reviewed changes

studavekar approved these changes Mar 20, 2018

View reviewed changes

0xc0170 approved these changes Mar 20, 2018

View reviewed changes

0xc0170 added ready for merge and removed needs: review labels Mar 20, 2018

0xc0170 added the release-version: 5.8.1 label Mar 20, 2018

0xc0170 merged commit 447e96e into ARMmbed:master Mar 20, 2018

0xc0170 removed the ready for merge label Mar 20, 2018

This was referenced Mar 23, 2018

Update USBTester product name #6439

Merged

Enable tickless mode on Silicon Labs targets #6475

Merged

cmonr mentioned this pull request Apr 3, 2018

Updated macro that disables flash cache test #6535

Closed

kjbracey mentioned this pull request Apr 27, 2018

Lora: Fix max tx power check #6740

Merged

Disables flash clock and cache test #6340

Disables flash clock and cache test #6340

Uh oh!

Conversation

cmonr commented Mar 12, 2018

Description

Pull request type

Uh oh!

cmonr commented Mar 12, 2018

Uh oh!

cmonr commented Mar 12, 2018

Uh oh!

mbed-ci commented Mar 13, 2018

Build : SUCCESS

Triggering tests

Uh oh!

mbed-ci commented Mar 13, 2018

Exporter Build : ABORTED

Uh oh!

studavekar Mar 13, 2018

Choose a reason for hiding this comment

Uh oh!

cmonr Mar 13, 2018

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Mar 13, 2018

Uh oh!

cmonr commented Mar 13, 2018

Uh oh!

0xc0170 commented Mar 13, 2018

Uh oh!

studavekar commented Mar 13, 2018

Uh oh!

cmonr commented Mar 13, 2018

Uh oh!

studavekar commented Mar 13, 2018

Uh oh!

cmonr commented Mar 16, 2018

Uh oh!

bridadan commented Mar 16, 2018

Uh oh!

bridadan commented Mar 16, 2018

Uh oh!

studavekar commented Mar 16, 2018

Uh oh!

SenRamakri commented Mar 16, 2018

Uh oh!

bridadan commented Mar 16, 2018

Uh oh!

cmonr commented Mar 16, 2018

Uh oh!

0xc0170 commented Mar 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

cmonr commented Mar 19, 2018

Uh oh!

theotherjimmy left a comment

Choose a reason for hiding this comment

Uh oh!

bridadan left a comment

Choose a reason for hiding this comment

Uh oh!

studavekar commented Mar 20, 2018

Uh oh!

mbed-ci commented Mar 20, 2018

Build : SUCCESS

Triggering tests

Uh oh!

mbed-ci commented Mar 20, 2018

Exporter Build : SUCCESS

Uh oh!

0xc0170 left a comment

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Mar 19, 2018 •

edited

Loading