Skip to content

tests/spiffs: Move from unittests to regular test#10210

Merged
cladmi merged 1 commit intoRIOT-OS:masterfrom
bergzand:pr/pkg/spiffs_move
May 28, 2019
Merged

tests/spiffs: Move from unittests to regular test#10210
cladmi merged 1 commit intoRIOT-OS:masterfrom
bergzand:pr/pkg/spiffs_move

Conversation

@bergzand
Copy link
Copy Markdown
Member

@bergzand bergzand commented Oct 19, 2018

Contribution description

another one related to #10187

Testing procedure

Verify that tests/pkg_spiffs works: BOARD=<your-favourite> make -C tests/pkg_spiffs flash test

Issues/PRs references

#10187

@bergzand bergzand added Area: tests Area: tests and testing framework Area: pkg Area: External package ports Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Oct 19, 2018
@bergzand bergzand mentioned this pull request Oct 19, 2018
Open
16 tasks
miri64
miri64 reviewed Nov 27, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested for native, samr21-xpro, and native. According to Murdock some BOARD_INSUFFIENT_MEMORY needs to be compiled.

tests/unittests/Makefile Outdated
z1 \
#
DISABLE_TEST_FOR_MSP430 := tests-relic tests-spiffs tests-cpp_%
DISABLE_TEST_FOR_MSP430 := tests-relic tests-cpp_%
Copy link
Copy Markdown
Member

@miri64 miri64 Nov 27, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This blacklisting needs to also be moved to tests/pkg_spiffs.

Copy link
Copy Markdown
Member Author

@bergzand bergzand May 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be okay now

@miri64 miri64 added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Nov 27, 2018
@cladmi cladmi added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR labels Nov 28, 2018
cladmi
cladmi suggested changes Nov 28, 2018
View reviewed changes
Copy link
Copy Markdown
Contributor

@cladmi cladmi left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested with IoT-LAB nodes (with the branch rebased on master)

BOARDS="samr21-xpro pba-d-01-kw2x b-l475e-iot01a nrf52dk arduino-zero nrf52840dk iotlab-m3 microbit frdm-kw41z"
for board in ${BOARDS}; do BOARD=${board} RIOT_CI_BUILD=1 IOTLAB_NODE=auto-ssh BUILD_IN_DOCKER=1 DOCKER="sudo docker" make --no-print-directory -C tests/pkg_spiffs clean all flash test || break; done

However for these boards, make test took around 6 seconds only, with the time after reset included, so the 120 looks like not needed. The default 10s could be enough.

Test does not compile for msp430-v1_3b due to not enough memory.

tests/pkg_spiffs/tests/01-run.py Outdated
board = os.environ['BOARD']
# Increase timeout on "real" hardware
timeout = 120 if board is not 'native' else -1
child.expect('OK \(\d+ tests\)', timeout=timeout)
Copy link
Copy Markdown
Contributor

@cladmi cladmi Nov 28, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also directly inline changing this regex to a raw string.

Suggested change
child.expect('OK \(\d+ tests\)', timeout=timeout)
child.expect(r'OK \(\d+ tests\)', timeout=timeout)

Copy link
Copy Markdown
Member Author

@bergzand bergzand May 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@bergzand bergzand force-pushed the pr/pkg/spiffs_move branch from cf1174b to f8a80b1 Compare May 28, 2019 14:10
cladmi
cladmi approved these changes May 28, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@cladmi cladmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test succeeded on IoT-LAB boards arduino-zero iotlab-m3 samr21-xpro b-l475e-iot01a microbit.

It also succeeded on arduino-mega2560 frdm-k64f frdm-kw41z msba2 nrf52dk nucleo-f103rb pba-d-01-kw2x sltb001a stm32f3discovery but currently failed on the mulle.
However mulle was not passing unittests in master either.

https://ci-ilab.imp.fu-berlin.de/job/experimental-pull-request-tests/22/consoleText

Please squash!

@bergzand
Copy link
Copy Markdown
Member Author

bergzand commented May 28, 2019

Please squash!

Mind if I update the board blacklist/insufficient memory list while squashing?

@cladmi
Copy link
Copy Markdown
Contributor

cladmi commented May 28, 2019

@bergzand no problem

@bergzand bergzand force-pushed the pr/pkg/spiffs_move branch from 2738789 to eb7d7d6 Compare May 28, 2019 15:41
@cladmi
Copy link
Copy Markdown
Contributor

cladmi commented May 28, 2019

All green, let's go. No more pkgs in tests/unittests.

@cladmi cladmi merged commit 7987767 into RIOT-OS:master May 28, 2019
@bergzand bergzand deleted the pr/pkg/spiffs_move branch May 28, 2019 17:24
@bergzand
Copy link
Copy Markdown
Member Author

bergzand commented May 28, 2019

Thank you!

@MrKevinWeiss MrKevinWeiss added the Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer label Jun 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miri64 miri64 miri64 left review comments

@jnohlgard jnohlgard Awaiting requested review from jnohlgard

@vincent-d vincent-d Awaiting requested review from vincent-d

+1 more reviewer

@cladmi cladmi cladmi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Area: pkg Area: External package ports Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: run tests If set, CI server will run tests on hardware for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bergzand @cladmi @miri64 @MrKevinWeiss