tests/riotboot_flashwrite: add python test script #11808

fjmolinas · 2019-07-05T16:04:36Z

Contribution description

This PR adds a test script for riotboot_flashwrite. ~~It it still missing a lot of dependencies PR's from @kaspar030.~~

~~NOTE: the autthorship in the first 6 commits is only @kaspar030 it's a residue of cherry-pick.~~

Testing procedure

Follow the README and test over ethos and wireless.

Issues/PRs references

Depends on ~~#11707~~, ~~#11690~~ and ~~#11816~~.

fjmolinas · 2019-07-15T17:56:20Z

@kaspar030 there are no dependencies anymore!

kaspar030

Having the suit testing in the queue, which implicitly tests riotboot_flashwrite, does it make sense to have another test application?

kaspar030 · 2019-07-16T06:57:20Z

tests/riotboot_flashwrite/Makefile

@@ -1,6 +1,8 @@
-DEVELHELP ?= 0
+# If no BOARD is found in the environment, use this default:


tests usually include ../Makefile.tests_common.
That sets BOARD and DEVELHELP to defaults, and also configures the application name to something that cannot clash. As is, this application would be called riotboot_flashwrite, same as the module.

It is still included I just moved it to the bottom. The first commit only does that, to group things better (although that is just my personal taste I guess), but I can always drop that commit.

Ahok. Then please move it back to the top, for consistency. Just put the variables that override it above.

fjmolinas · 2019-07-16T07:40:05Z

Having the suit testing in the queue, which implicitly tests riotboot_flashwrite, does it make sense to have another test application?

IMO although suit implicitly tests riotboot_flashwrite, having an explicit test is better. Tests that cover specific or smaller building block are needed even if there is an integration test that covers the whole system. Following that argument why would I have a test for sx1276 when semtech_loramac already tests that. If I'm testing suit for a platform if see the test fails I can go back on the building blocks, does tests/riotboot succeed? does tests/riotboot_flashwrite succeed? IMO being able to isolate things helps with testing and finding bugs.

Also, although suit attempts to be a standard, it might not be used, people might still implement there own OTA mechanism, because they want something smaller/simplet, or because they think having something privative is more secure (I'm not stating an opinion here), or whatever reason. I think saying a suit test is enough is presuming on what a user or developer might want to do with riotboot_flashwrite or riotboot.

I also think that running tests/riotboot_flashwrite should be part of the standard testing a user does when implementing riotboot for a board (if it supports it). tests/riotboot could succeed and tests/riotboot_flashwrite fail because of alignment issues between the slots/bootloader and the flashpage_size, etc.

And as a developer I find it useful do have an automatic script I can launch whenever I change riotboot_flashwrite (completely biased opinion :) ). EDIT: and makes review process easier for the reviewer.

kaspar030 · 2019-07-16T07:52:31Z

BTW, if you'd like to test just this test on murdock without rebuilding, you can add a "REMOVEME" commit that adds a filter to "./.murdock", somewhere at the top:

BOARDS=samr21-xpro
APPS=tests/riotboot_flashwrite

That should limit the build to that one application/board.

kaspar030 · 2019-07-16T07:53:09Z

And as a developer I find it useful do have an automatic script I can launch whenever I change riotboot_flashwrite (completely biased opinion :) ).

OK! (I just saw like 80% duplication in Makefile and test...)

kaspar030 · 2019-07-16T07:54:18Z

(I just saw like 80% duplication in Makefile and test...)

(and the flash/reflash tests over network block the board for >two minutes, compared to ~20sec for a regular test)

fjmolinas · 2019-07-16T08:00:06Z

(and the flash/reflash tests over network block the board for >two minutes, compared to ~20sec for a regular test)

OK! (I just saw like 80% duplication in Makefile and test...)

Yes I know... :( maybe the solution could be to modify the suit test.. so it proceeds with the update even if some part of suit verification fails? The test scipt would fail but only on the suit part and not the flashwrite part?

fjmolinas · 2019-07-16T08:00:44Z

BTW, if you'd like to test just this test on murdock without rebuilding, you can add a "REMOVEME" commit that adds a filter to "./.murdock", somewhere at the top:

BOARDS=samr21-xpro
APPS=tests/riotboot_flashwrite
That should limit the build to that one application/board.

Cool! Thanks!

aabadie

Looks good but would need a rebase to get the latest development made in the test scripts behavior recently (test_interactive_sync).

aabadie · 2020-01-12T16:15:42Z

tests/riotboot_flashwrite/Makefile

+# create final binaries with specific APP_VER values. The CI RasPi test workers
+# don't compile themselves, thus add the required files here so they will be
+# submitted along with the test jobs.
+TEST_EXTRA_FILES=$(SLOT_RIOT_ELFS)


Suggested change

TEST_EXTRA_FILES=$(SLOT_RIOT_ELFS)

TEST_EXTRA_FILES = $(SLOT_RIOT_ELFS)

aabadie · 2020-01-12T16:16:50Z

tests/riotboot_flashwrite/main.c

-    printf("Current slot=%d\n", current_slot);
-    riotboot_slot_print_hdr(current_slot);
+    if (current_slot != -1) {
+        /* Sometimes, udhcp output messes up the following printfs.  That


Suggested change

/* Sometimes, udhcp output messes up the following printfs. That

/* Sometimes, udhcp output messes up the following printfs. That

aabadie · 2020-01-12T16:20:50Z

tests/riotboot_flashwrite/tests/01-run.py

+        "-b",
+        "64",
+    ]
+    subprocess.Popen(cmd, cwd=BINDIR)


Why not :

Suggested change

subprocess.Popen(cmd, cwd=BINDIR)

subprocess.call(cmd, cwd=BINDIR)

?
with eventually the use of an assert.

aabadie · 2020-01-12T16:22:27Z

tests/riotboot_flashwrite/tests/01-run.py

+    """For one board test if specified application is updatable"""
+
+    # Initial Setup and wait for address configuration
+    child.expect_exact("main(): This is RIOT!")


This is not needed now with the test_interactive_sync module.

Co-authored-by: Kaspar Schleiser <kaspar@schleiser.de>

fjmolinas · 2020-01-13T17:17:58Z

Rebased but there are some issue with the script will address later.

stale · 2020-07-17T07:50:07Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

aabadie · 2020-07-17T07:57:48Z

still useful but might need a refresh

fjmolinas · 2020-07-24T15:27:29Z

Staled, doesn't work anymore.

aabadie · 2020-07-24T15:29:35Z

Staled, doesn't work anymore.

What is not working ?

fjmolinas · 2020-07-24T15:34:10Z

Staled, doesn't work anymore.

What is not working ?

See #11808 (comment)

aabadie · 2020-07-24T15:35:15Z

See #11808 (comment)

You mean "there are some issue with the script will address later." This is a bit vague ;)

fjmolinas added Area: tests Area: tests and testing framework State: waiting for other PR State: The PR requires another PR to be merged first Area: OTA Area: Over-the-air updates labels Jul 5, 2019

fjmolinas force-pushed the pr_riotboot_flashwrite_test branch from e2af9cc to b74417c Compare July 8, 2019 12:13

This was referenced Jul 8, 2019

dist/tools/ethos: add setup_network.sh script #11816

Merged

riotboot/flashwrite: use chunks instead of pages, support flashpage_write_raw #11705

Closed

fjmolinas force-pushed the pr_riotboot_flashwrite_test branch from b74417c to 48468bb Compare July 15, 2019 15:08

fjmolinas removed the State: waiting for other PR State: The PR requires another PR to be merged first label Jul 15, 2019

fjmolinas force-pushed the pr_riotboot_flashwrite_test branch 5 times, most recently from 46f4285 to b05e98b Compare July 15, 2019 16:14

fjmolinas force-pushed the pr_riotboot_flashwrite_test branch 2 times, most recently from 5033f8e to f267f5f Compare July 15, 2019 18:25

fjmolinas requested review from kaspar030 and aabadie July 16, 2019 06:36

kaspar030 requested changes Jul 16, 2019

View reviewed changes

This was referenced Jul 24, 2019

cpu/stm32f7: add riotboot requirements #11899

Merged

cpu/stm32f2: add riotboot requirements #11900

Merged

fjmolinas mentioned this pull request Aug 7, 2019

riotnode: node abstraction package #10949

Closed

gschorcht mentioned this pull request Nov 18, 2019

doc: Fix cpu specific enum in doxygen #12737

Merged

fjmolinas mentioned this pull request Jan 6, 2020

dist/tools/ci: print toolchain version update for esp8266 #13027

Merged

aabadie requested changes Jan 12, 2020

View reviewed changes

fjmolinas and others added 10 commits January 13, 2020 17:49

tests/riotboot_flashwrite: cleanup Makefile

c5a8f70

tests/riotboot_flashwrite: use nrfmin if BOARD=nrf52dk

323889e

tests/riotboot_flashwrite: add ethos config for local test

968f57e

tests/riotboot_flashwrite: print current APP_VER

d208f85

tests/riotboot_flashwrite: add python test script

d90dfa1

Co-authored-by: Kaspar Schleiser <kaspar@schleiser.de>

tests/riotboot_flashwrite: add TEST_EXTRA_FILES

e22ea85

fixup! tests/riotboot_flashwrite: cleanup Makefile

0197814

fixup! tests/riotboot_flashwrite: cleanup Makefile

12583f2

fixup! tests/riotboot_flashwrite: print current APP_VER

a68a0d9

fixup! tests/riotboot_flashwrite: add python test script

b6b20f4

fjmolinas force-pushed the pr_riotboot_flashwrite_test branch from 23b3e27 to b6b20f4 Compare January 13, 2020 17:17

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Jul 17, 2020

stale bot removed the State: stale State: The issue / PR has no activity for >185 days label Jul 17, 2020

fjmolinas closed this Jul 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests/riotboot_flashwrite: add python test script #11808

tests/riotboot_flashwrite: add python test script #11808

fjmolinas commented Jul 5, 2019 •

edited

Loading

fjmolinas commented Jul 15, 2019

kaspar030 left a comment

kaspar030 Jul 16, 2019

fjmolinas Jul 16, 2019

kaspar030 Jul 16, 2019

fjmolinas Jul 16, 2019

fjmolinas commented Jul 16, 2019 •

edited

Loading

kaspar030 commented Jul 16, 2019

kaspar030 commented Jul 16, 2019

kaspar030 commented Jul 16, 2019

fjmolinas commented Jul 16, 2019

fjmolinas commented Jul 16, 2019

aabadie left a comment

aabadie Jan 12, 2020

aabadie Jan 12, 2020

aabadie Jan 12, 2020

aabadie Jan 12, 2020

fjmolinas commented Jan 13, 2020

stale bot commented Jul 17, 2020

aabadie commented Jul 17, 2020

fjmolinas commented Jul 24, 2020

aabadie commented Jul 24, 2020

fjmolinas commented Jul 24, 2020

aabadie commented Jul 24, 2020

		@@ -1,6 +1,8 @@
		DEVELHELP ?= 0
		# If no BOARD is found in the environment, use this default:

	TEST_EXTRA_FILES=$(SLOT_RIOT_ELFS)
	TEST_EXTRA_FILES = $(SLOT_RIOT_ELFS)

	/* Sometimes, udhcp output messes up the following printfs. That
	/* Sometimes, udhcp output messes up the following printfs. That

	subprocess.Popen(cmd, cwd=BINDIR)
	subprocess.call(cmd, cwd=BINDIR)

tests/riotboot_flashwrite: add python test script #11808

tests/riotboot_flashwrite: add python test script #11808

Conversation

fjmolinas commented Jul 5, 2019 • edited Loading

Contribution description

Testing procedure

Issues/PRs references

fjmolinas commented Jul 15, 2019

kaspar030 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjmolinas commented Jul 16, 2019 • edited Loading

kaspar030 commented Jul 16, 2019

kaspar030 commented Jul 16, 2019

kaspar030 commented Jul 16, 2019

fjmolinas commented Jul 16, 2019

fjmolinas commented Jul 16, 2019

aabadie left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fjmolinas commented Jan 13, 2020

stale bot commented Jul 17, 2020

aabadie commented Jul 17, 2020

fjmolinas commented Jul 24, 2020

aabadie commented Jul 24, 2020

fjmolinas commented Jul 24, 2020

aabadie commented Jul 24, 2020

fjmolinas commented Jul 5, 2019 •

edited

Loading

fjmolinas commented Jul 16, 2019 •

edited

Loading