Wifi connection success validation #445
Conversation
|
@Frijol run |
|
Actually, looks like there are no lint nits. I think you can just do |
|
An jscs errors will have to be fixed manually |
| // End the connection | ||
| return self.connection.end() | ||
| .then(resolve); | ||
| }); |
There was a problem hiding this comment.
You can replace lines 102-115 with the receive method:
self.receive(remoteProcess).then(function(data) {
// do the check for ifup etc here, then just...
// data is already a string
return resolve();
}).catch(function(error) {
logs.err('Error connecting:', error); // already a string
// this is sufficient, all connections will be closed by the process that called this operation
return resolve();
});
|
fixed merge conflicts and delinted most of the way, but it stopped here: Testing usb_connection.js........OK
Testing update.js.............OK
Testing wifi.js.....F..
>> Tessel.prototype.connectToNetwork - properCredentials
>> Error: 2 == 1
>> at /Users/kelseybreseman/Code/t2-cli/test/unit/wifi.js:195:14
>> Tessel.prototype.connectToNetwork - properCredentials
fix merge conflicts and delint
>> Error: 2 == 1
>> at /Users/kelseybreseman/Code/t2-cli/test/unit/wifi.js:196:14
Warning: 2/641 assertions failed (26437ms) Use --force to continue.
Aborted due to warnings.@rwaldron I'm not that familiar with grunt, should I force? What does that do? Also, need to test this with a tessel |
|
also, will fix re your other note too. |
| // End the connection | ||
| return self.connection.end() | ||
| .then(resolve); | ||
| }); |
There was a problem hiding this comment.
This change is missing: #445 (comment)
|
So, the failures you're seeing are actual broken tests that you need to resolve: I will take a look and see if I can help iron those out. |
| @@ -98,8 +98,43 @@ Tessel.prototype.connectToNetwork = function(opts) { | |||
| }) | |||
| .then(function() { | |||
| // Then set the new credientials | |||
| logs.info('Credentials set!'); | |||
| resolve(); | |||
| return self.connection.exec(commands.commitWirelessCredentials()); | |||
There was a problem hiding this comment.
This the source of the one of the test failures—commitWirelessCredentials has already been called by setWifiState just before this. If you remove the entire then, that test no longer fails.
Edit: I think it would be smarter to replace the call to setWifiState (above) with self.simpleExec(commands.turnOnWifi(enable)). Then this change stays as is.
|
tested on T2, didn't work as hoped, revised. The takeaway: ubus is different from other commands we run on T2. It doesn't work to use a .receive, and the remote process has to be closed specifically. Works great now (thanks for the help @johnnyman727). Ready to merge! |
Frijol
changed the title
|
Ah, interesting—thanks for pointing that out. This patch also needs tests to prove the functionality—@johnnyman727 can you help @Frijol get started on those? |
|
@johnnyman727 I've started an update to this to work with #471 and the refactoring that happened with the |
|
@HipsterBrown in theory, we could just merge this branch since it's been reviewed and has tests. The tests I added shouldn't break any of the tests that your branch introduces, right? |
|
@johnnyman727 Well my PR branch for #471 changes most of the |
|
@HipsterBrown no worries if you have to throw theses tests away. |
|
@johnnyman727 Ok, well they really helped show how to mock those remote process events. 👍 |
|
👍 Glad they were helpful! |
|
We're going to do tests in a follow up |
Wifi connection success validation
Working on #395
The current implementation is functional, but:
Other open questions:
ubus listen, do I need to close that, or does it get closed when the connection is resolved?@rwaldron comments appreciated. Thanks to @kevinmehall for the docs links.