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.

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

Fixes #2197 and #2195: Refresh admin page keeps it at same location; Add auto-scroll to show output in jobs tab. #2218

Merged
merged 4 commits into from
Jul 6, 2016

Conversation

kevintab95
Copy link
Member

Fixes #2197 and #2195: Refresh admin page keeps it at the same location; Auto-scroll for viewing output in jobs tab in admin panel.

@kevintab95 kevintab95 changed the title Fixes #2197 and #2195: Refresh admin page keeps it at same location; Add auto-scroll to show output in jobs tab in admin panel. Fixes #2197 and #2195: Refresh admin page keeps it at same location; Add auto-scroll to show output in jobs tab. Jul 4, 2016
@seanlip
Copy link
Member

seanlip commented Jul 5, 2016

@kevintab95 -- FYI, the Travis tests are failing!

@codecov-io
Copy link

codecov-io commented Jul 5, 2016

Current coverage is 48.01%

Merging #2218 into develop will decrease coverage by 0.06%

@@            develop      #2218   diff @@
==========================================
  Files           185        185          
  Lines         15836      15857    +21   
  Methods        2718       2720     +2   
  Messages          0          0          
  Branches       2688       2692     +4   
==========================================
  Hits           7613       7613          
- Misses         8223       8244    +21   
  Partials          0          0          

Powered by Codecov. Last updated by 54e5a49...0762dec

@kevintab95
Copy link
Member Author

The e2e test for the library suite passes on my local machine but fails the travis test. Can someone restart build?

return window.location.hash;
}, function(newHash) {
switch (newHash) {
case '#jobs':
Copy link
Contributor

@wxyxinyu wxyxinyu Jul 6, 2016

Choose a reason for hiding this comment

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

Can we pull out these URLs as constants, and use them in the HTML code too (you'll need to change to using ng-href)? This would help prevent people from changing the html and breaking the tab reloading.

@wxyxinyu
Copy link
Contributor

wxyxinyu commented Jul 6, 2016

@kevintab95 -- Thanks! I left some comments.

@kevintab95
Copy link
Member Author

@wxyxinyu I've made some changes like you suggested. Thanks!

@@ -27,9 +27,32 @@ oppia.controller('Admin', ['$scope', '$http', function($scope, $http) {
$scope.TAB_JOBS = 'TAB_JOBS';
$scope.TAB_CONFIG = 'TAB_CONFIG';
$scope.TAB_MISC = 'TAB_MISC';
$scope.jobsUrl = '#jobs';
Copy link
Contributor

Choose a reason for hiding this comment

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

The convention is to use all caps for constants, and you should do so here.

@wxyxinyu
Copy link
Contributor

wxyxinyu commented Jul 6, 2016

Just one small thing.

This is great, it has been a bit annoying on the admin page for a while. Thanks for doing this!

@kevintab95
Copy link
Member Author

The e2e test for misc suite passes on my local machine but fails the Travis test, please restart the build. I wonder why this keeps happening?

@wxyxinyu
Copy link
Contributor

wxyxinyu commented Jul 6, 2016

I think it's a spurious error. I restarted the test, hopefully it passes this time.

The travis tests are sometimes a bit flaky haha. There are some issues tracking attempts to make tests less flaky e.g. #2042

@kevintab95
Copy link
Member Author

Oh OK, thanks! and actually I noticed that it failed on my machine when i ran the test without the --sharding=false option. It took a lot longer when I used that option though, but in the end all the tests were successful.
IDK if that helps, its just an observation.

@wxyxinyu
Copy link
Contributor

wxyxinyu commented Jul 6, 2016

Yeah, sharding does lead to some flakyness, because the CPU load causes the e2e tests to run slower in some cases, and this causes some timeouts or clicks not being registered in the right place. But I generally think sharding is worth it, just because running without sharding takes 1h+ for all the e2e tests.

The wiki page here https://github.com/oppia/oppia/wiki/Running-Tests#end-to-end-tests does describe some of the common problems and suggested solutions (especially the common e2e error messages that may indicate tests failed spuriously).

@wxyxinyu wxyxinyu merged commit 8486f1a into oppia:develop Jul 6, 2016
@kevintab95
Copy link
Member Author

Oh alright, cool!
Thanks!

wxyxinyu pushed a commit that referenced this pull request Aug 8, 2016
…Add auto-scroll to show output in jobs tab. (#2218)

* Fixes #2197 and fixes #2195

* fix e2e error (library)

* Use URL as constants

* Constants in all caps
ctao5660 pushed a commit to ctao5660/oppia that referenced this pull request Nov 23, 2018
…location; Add auto-scroll to show output in jobs tab. (oppia#2218)

* Fixes oppia#2197 and fixes oppia#2195

* fix e2e error (library)

* Use URL as constants

* Constants in all caps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants