Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Basic test environment setup #23

Merged
merged 24 commits into from
May 29, 2017
Merged

Basic test environment setup #23

merged 24 commits into from
May 29, 2017

Conversation

princiya
Copy link
Collaborator

@princiya princiya commented May 24, 2017 •
edited
Loading

  • As pointed out in this issue Testing platforms #19, the goal is to move non-functional code into a test directory.
  • Using this example as a reference, I have setup a basic test environment which includes - Karma, Mocha and Chai.
  • get methods from js/store.js are now testable.
  • The store object is mocked and the corresponding methods are tested.

To test this PR:
npm install
npm run test

Screenshot

test-success-1

princiya
princiya commented May 24, 2017
View reviewed changes
js/store.js Outdated
@@ -3,6 +3,10 @@ const store = {
this.addListeners();
},

getEmptyObject() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added for testing purpose. once I have sinon-chrome setup working, this function will be removed.

princiya
princiya commented May 24, 2017
View reviewed changes
js/store.js Outdated
@@ -59,6 +63,7 @@ const store = {
}
};

/*
store.getAll()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The goal of this PR is to move this code into the test directory

Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

jonathanKingston
jonathanKingston suggested changes May 25, 2017
View reviewed changes
Copy link
Contributor

@jonathanKingston jonathanKingston left a comment

Choose a reason for hiding this comment

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

Are we ready to merge this yet?

.gitignore
@@ -1 +1,2 @@
node_modules
node_modules
npm-debug.log
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

js/store.js Outdated
@@ -59,6 +63,7 @@ const store = {
}
};

/*
store.getAll()
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

package.json Outdated
},
"repository": {
"type": "git",
"url": "git+https://github.com/pauljt/lightbeam-we.git"
Copy link
Contributor

Choose a reason for hiding this comment

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

Bye bye @pauljt you will be missed!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

haha... yeah ;)

@princiya
Copy link
Collaborator Author

I am yet to add sinon-chrome part. maybe I do that in another PR? if so, this can be merged

@groovecoder
Copy link
Member

Nice start on the testing harness! eslint doesn't like a lot of stuff here, and I don't think eslint comes pre-configured to deal with .json files?

@princiya
Copy link
Collaborator Author

@groovecoder there is an eslint-plugin-json, eslint-plugin-mocha to fix the existing lint errors. #TIL

@jonathanKingston
Copy link
Contributor

@princiya Lots of ESLint errors in Travis to fix:

https://travis-ci.org/electrolyfish/lightbeam-we/builds/236005297?utm_source=github_status&utm_medium=notification

Could you look into those?

@princiya
Copy link
Collaborator Author

Yes, will do. Back to DO NOT MERGE :)

@princiya princiya changed the title Basic test environment setup [WIP] Basic test environment setup May 26, 2017
@princiya princiya changed the title [WIP] Basic test environment setup Basic test environment setup May 26, 2017
princiya
princiya commented May 26, 2017
View reviewed changes
js/background.js
// When the user clicks browserAction icon in toolbar, run Lightbeam
browser.browserAction.onClicked.addListener(runLightbeam);

capture.init();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@biancadanforth are you able to invoke capture.init() from here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@princiya Yep. I've tested it in the browser.

@jonathanKingston jonathanKingston merged commit 9f8680c into master May 29, 2017
@princiya princiya deleted the test-setup branch May 29, 2017 15:29
@biancadanforth biancadanforth mentioned this pull request May 30, 2017
Closed
@princiya princiya mentioned this pull request Aug 23, 2017
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@biancadanforth biancadanforth biancadanforth left review comments

@jonathanKingston jonathanKingston jonathanKingston requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@princiya @groovecoder @jonathanKingston @biancadanforth