New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement functional Wifi tests #5161

Merged
merged 8 commits into from Oct 9, 2017

Conversation

Projects
None yet
6 participants
@SeppoTakalo
Contributor

SeppoTakalo commented Sep 21, 2017

Description

Implement 100% function coverage for WifiInterface as specified
in "Wifi test plan"

Idea is to have full coverage of WiFiInterface API callbacks. Does not test any of the socket functionality, those will be provided later, perhaps within #4795

Uses idea of separating the code which detects the interface into one singe file get_interface.c and its callback get_interface() which always gives a new initialised interface and destroys the previous one. Assumption is that destructor should clean up the driver state and constructor should bring it in sane condition. That allows us to write all test cases separately, everyone within their own file for easy maintain.

Status

IN DEVELOPMENT

Verified in ESP8266, does not pass without fixing the driver.

Missing the example mbed_app.json and some description about the test environment.

#include "mbed.h"
#include "ESP8266Interface.h"

WiFiInterface *get_interface()

This comment has been minimized.

@sarahmarshy

sarahmarshy Sep 21, 2017

Contributor

Do you think you could use the JSON files introduced in #4795 for this functionality?

The idea is that you have a JSON file describing how to instantiate the interface. That file is mapped to from some keywords. And each target has keywords it supports.

Here is an example of how the test configuration files are used in a test.

If the tests are not target specific or the interface is outside of mbed OS, then you can create a new config file outside of mbed OS and use it in the test command like. mbed test -m [mcu] -t [toolchain] --test-config path/outside/mbed

This comment has been minimized.

@SeppoTakalo

SeppoTakalo Sep 21, 2017

Contributor

I could, these two approaches are doing the same effect.

I can rebase this PR and take your changes once you are in master.

This comment has been minimized.

@SeppoTakalo

SeppoTakalo Oct 3, 2017

Contributor

Actually, now after review I noticed that the --test-config overrides the --app-config so I cannot use it.
We need app-config to deliver parameters like Wifi passwords, etc, for test cases.

@SeppoTakalo SeppoTakalo force-pushed the SeppoTakalo:wifi_tests branch Sep 29, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Sep 29, 2017

@SeppoTakalo Who do you think should review these besides @sarahmarshy ?

@kjbracey-arm @KariHaapalehto ?

@KariHaapalehto

help text for "wifi-secure-ssid" and "wifi-unsecure-ssid" are same, should they specify the secure/unsecure?

Lot of compile warnings:
06:31:14 Compile [ 5.6%]: get_interface.cpp
06:31:14 [Warning] OdinWiFiInterface.h@54,0: #1300-D: ~OdinWiFiInterface inherits implicit virtual
06:31:14 Compile [ 11.1%]: main.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 16.7%]: wifi-constructor.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 22.2%]: wifi_connect.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 27.8%]: wifi_connect_disconnect_repeat.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 33.3%]: wifi_connect_nocredentials.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 38.9%]: wifi_connect_params_channel.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 44.4%]: wifi_connect_params_channel_fail.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 50.0%]: wifi_connect_params_null.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 55.6%]: wifi_connect_params_valid_secure.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 61.1%]: wifi_connect_params_valid_unsecure.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 66.7%]: wifi_connect_secure.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 72.2%]: wifi_connect_secure_fail.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 77.8%]: wifi_get_rssi.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 83.3%]: wifi_scan.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 88.9%]: wifi_scan_null.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [ 94.4%]: wifi_set_channel.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline
06:31:14 Compile [100.0%]: wifi_set_credential.cpp
06:31:14 [Warning] wifi_tests.h@26,0: #1-D: last line of file ends without a newline

@KariHaapalehto

If the proper app_config file isn't defined, the compiling of the wifi test will fail. Build shouldn't fail, it should be skipped.

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Oct 3, 2017

@KariHaapalehto Fixed now. Looks like its building OK.

@KariHaapalehto

Looks ok to me

@SeppoTakalo SeppoTakalo removed request for bulislaw and adbridge Oct 3, 2017

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Oct 3, 2017

@0xc0170 These are testcases defined in our internal test plan, reviewer should be aware of those.
Therefore KariHaapalehto was the correct person to review.

TESTS/network/wifi/wifi_tests.h Outdated
WiFiInterface *get_interface(void);

/* Test cases */
void wifi_constructor(void);

This comment has been minimized.

@0xc0170

0xc0170 Oct 4, 2017

Member

To be aware, we are enabling doxygen generation for tests (#5238). It would be beneficial to have these test cases documented here

@@ -0,0 +1,26 @@
#ifndef WIFI_TESTS_H

This comment has been minimized.

@0xc0170

0xc0170 Oct 4, 2017

Member

I believe every file in the codebase should have license header on the top, even test files.

TESTS/network/wifi/wifi_tests.h Outdated
void wifi_scan_null(void);
void wifi_scan(void);

#endif //WIFI_TESTS_H

This comment has been minimized.

@0xc0170

0xc0170 Oct 4, 2017

Member

does not this produce a warning (no new line at the end of the file) in ARMCC ? It should have or github is not showing it correctly here?

@0xc0170 0xc0170 added the needs: CI label Oct 4, 2017

SeppoTakalo added some commits Sep 19, 2017

Set timeout to 4 minutes.
Wifi connections and scanning takes long time. 2 minutes might not be enough.
Implement functional Wifi tests
Implement 100% function coverage for WifiInterface as specified
in "Wifi test plan"
Fix includes.
Cannot include header file witin a function (without severe side effects)

@SeppoTakalo SeppoTakalo force-pushed the SeppoTakalo:wifi_tests branch to 80198f2 Oct 4, 2017

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Oct 4, 2017

  • Updated based on @0xc0170 feedback.
  • Rebased on top of master.
@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 4, 2017

What about the license headers in these new files?

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Oct 4, 2017

Oops.. Good catch.

I updated only Doxygen tags and newline error.

@0xc0170

0xc0170 approved these changes Oct 5, 2017

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 5, 2017

restarted travis (it failed due to time limit, not certain why so restarted), who else should review @SeppoTakalo ? Ready for CI?

@SeppoTakalo

This comment has been minimized.

Contributor

SeppoTakalo commented Oct 5, 2017

I think we are good to go if Kari have reviewed.
These are testcases, implemented according to our plan, so do not need a large review round.

@0xc0170

This comment has been minimized.

Member

0xc0170 commented Oct 7, 2017

/morph test-nightly

@mbed-bot

This comment has been minimized.

mbed-bot commented Oct 8, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 1542

All builds and test passed!

@0xc0170 0xc0170 added ready for merge and removed needs: CI labels Oct 9, 2017

@theotherjimmy theotherjimmy merged commit 42532d7 into ARMmbed:master Oct 9, 2017

4 checks passed

Cam-CI uvisor Build & Test Success
Details
ci/morph-test-nightly Job has completed
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@sarahmarshy

This comment has been minimized.

Contributor

sarahmarshy commented Oct 9, 2017

@SeppoTakalo, I was out of town, so I couldn't review. Does it make sense to make a PR to port these tests to use this style - #4795? We now have two ways of accomplishing the same thing. CC @theotherjimmy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment