Skip to content

General Cleanup (Order: 1)#215

Merged
rwaldron merged 1 commit intomasterfrom
jon-general-cleanup
Jul 22, 2015
Merged

General Cleanup (Order: 1)#215
rwaldron merged 1 commit intomasterfrom
jon-general-cleanup

Conversation

@johnnyman727
Copy link
Copy Markdown
Contributor

Goal: fix #104.

This PR will also add some better debugging capabilities for t2 list. You'll just need to type DEBUG=discovery:* t2 list or DEBUG=discovery:lan t2 list or DEBUG=discovery:usb t2 list.

@rwaldron
Copy link
Copy Markdown
Contributor

@johnnyman727 can you clean up these lint nits: https://travis-ci.org/tessel/t2-cli/builds/71706371 Thanks!!

@johnnyman727
Copy link
Copy Markdown
Contributor Author

@rwaldron yep! I didn't get around to cleaning it up yet.

@johnnyman727
Copy link
Copy Markdown
Contributor Author

@rwaldron any idea why the tests are hitting this error?

Testing lan_connection.js........F
>> LAN.Scanner.prototype.start - updateDiscoveredThrows
>> Error: 'Cannot read property \'fullname\' of undefined' == 'get outta here!'
>> at null.<anonymous> (test/unit/lan_connection.js:160:12)
>> at emit (events.js:107:17)
>> at /Users/Jon/Work/technical/t2-cli/lib/lan_connection.js:166:16
>> at process._tickCallback (node.js:355:11)

I couldn't understand how it's supposed to work.

@rwaldron
Copy link
Copy Markdown
Contributor

@johnnyman727 I think this needs to be broken up into different PRs—it's really hard to grok these changes altogether like this.

@rwaldron
Copy link
Copy Markdown
Contributor

The broken test was ensuring that scanner errors could be caught with an error handler: 2230b8e#diff-a9a705c264927afb490d6ba8a1d038faR100

@rwaldron
Copy link
Copy Markdown
Contributor

The issue is caused by https://github.com/tessel/t2-cli/pull/215/files#diff-e49c18af28b7866f1c40c82d8451cffbR155 because the test sends no arguments when the update event is emitted: https://github.com/tessel/t2-cli/blob/master/test/unit/lan_connection.js#L157 so the wrong error is thrown, because data is undefined and therefore has no fullname property.

@johnnyman727 johnnyman727 force-pushed the jon-general-cleanup branch from 977f299 to a294dc6 Compare July 22, 2015 02:56
@johnnyman727
Copy link
Copy Markdown
Contributor Author

Fixed, thanks @rwaldron!

@johnnyman727 johnnyman727 force-pushed the jon-general-cleanup branch from a294dc6 to bcab8d8 Compare July 22, 2015 03:22
@johnnyman727 johnnyman727 changed the title [WIP] General Cleanup General Cleanup Jul 22, 2015
@johnnyman727
Copy link
Copy Markdown
Contributor Author

Ready for review.

@johnnyman727 johnnyman727 changed the title General Cleanup General Cleanup (Order: 1) Jul 22, 2015
@rwaldron
Copy link
Copy Markdown
Contributor

  • Ran unit tests locally
  • Smoke test:
rwaldron at nova in ~/clonez/t2-cli on jon-general-cleanup
$ t2 list
INFO Searching for nearby Tessels...
  arnold  USB
rwaldron at nova in ~/clonez/t2-cli on jon-general-cleanup
$ t2 rename johnconnor
INFO Connecting to Tessel...
INFO Connected to arnold over LAN
INFO Changed name of device arnold to johnconnor
rwaldron at nova in ~/clonez/t2-cli on jon-general-cleanup
$ t2 rename arnold
INFO Connecting to Tessel...
INFO Connected to johnconnor over LAN
INFO Changed name of device johnconnor to arnold
rwaldron at nova in ~/clonez/t2-cli on jon-general-cleanup
$ cd resources/
rwaldron at nova in ~/clonez/t2-cli/resources on jon-general-cleanup
$ t2 run index.js
INFO Connecting to Tessel...
INFO Connected to arnold over LAN
INFO Writing index.js to RAM on arnold (60956.16 kB)...
INFO Deployed.
INFO Running index.js...
^C

LGTM.

rwaldron added a commit that referenced this pull request Jul 22, 2015
@rwaldron rwaldron merged commit f99ad94 into master Jul 22, 2015
@Frijol Frijol deleted the jon-general-cleanup branch August 19, 2015 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

General CLI code refactoring and cleanup

2 participants