Skip to content

ZEPPELIN 228 - Add unit tests for angular controllers #250

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
Closed
r-kamath wants to merge 4 commits into from
Closed

ZEPPELIN 228 - Add unit tests for angular controllers #250

r-kamath wants to merge 4 commits into from

Conversation

@r-kamath
Copy link
Member

@r-kamath r-kamath commented Aug 24, 2015

No description provided.

@Leemoonsoo
Copy link
Member

Leemoonsoo commented Sep 4, 2015

Looks good to me!

corneadoug
corneadoug reviewed Sep 5, 2015
View reviewed changes
zeppelin-web/test/spec/controllers/notebook.js
Copy link
Contributor

@corneadoug corneadoug Sep 5, 2015

Choose a reason for hiding this comment

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

To really test killSaveTimer(), startSaveTimer() should be called first since by default scope.saveTimer is null

@corneadoug
Copy link
Contributor

corneadoug commented Sep 5, 2015

Thanks, Overall looks good.
It's a good starting point

@Madhuka
Copy link
Contributor

Madhuka commented Sep 6, 2015

Happy to see people writing tests after it is been fixed.
@r-kamath : How is the test coverage for controllers?
It is good to have plugging for grunt to see the test coverage. It will reduce the test duplication.

Madhuka
Madhuka reviewed Sep 6, 2015
View reviewed changes
zeppelin-web/test/spec/controllers/paragraph.js
Copy link
Contributor

@Madhuka Madhuka Sep 6, 2015

Choose a reason for hiding this comment

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

It is good test functionality of the function, rather only checking is function is define.

In paragraph ctrl you will find lot functions and checking them all will not be easy.
There for you can test for use cases

Copy link
Member Author

@r-kamath r-kamath Sep 7, 2015

Choose a reason for hiding this comment

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

@Madhuka. will be adding more tests to check the functionality. this is just an initial test to check for fn defined.

Copy link
Member Author

@r-kamath r-kamath Sep 7, 2015

Choose a reason for hiding this comment

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

will file a JIRA for the test coverage grunt task

@corneadoug corneadoug mentioned this pull request Sep 6, 2015
Closed
@asfgit asfgit closed this in 0b47cde Sep 6, 2015
Leemoonsoo pushed a commit to Leemoonsoo/zeppelin that referenced this pull request Sep 17, 2015
@r-kamath @Leemoonsoo
ZEPPELIN 228 - Add unit tests for angular controllers
495739c 
Author: Renjith Kamath <[email protected]>

Closes apache#250 from r-kamath/ZEPPELIN-228 and squashes the following commits:

6735aac [Renjith Kamath] Merge branch 'master' into ZEPPELIN-228
743a13d [Renjith Kamath] ZEPPELIN-228 add tests for paragraph controller and update notebook tests
a750a30 [Renjith Kamath] ZEPPELIN-228 add more tests for getCronOptionNameFromValue
48bfb95 [Renjith Kamath] ZEPPELIN-228 add unit tests for controllers

(cherry picked from commit 0b47cde)
Signed-off-by: Lee moon soo <[email protected]>
lelou6666 pushed a commit to lelou6666/incubator-zeppelin that referenced this pull request Mar 25, 2016
@Leemoonsoo
Merge pull request apache#250 from NFLabs/fix-indent-html
11ad2ce 
fix indent
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

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

@r-kamath @Leemoonsoo @corneadoug @Madhuka