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

Improving the dev workflow: Setup browserstack, add necessary configuration files and tests for mobile #5340

Merged
merged 108 commits into from
Aug 4, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
108 commits
Select commit Hold shift + click to select a range
59adedb
Basic Setup
apb7 Jul 19, 2018
a785896
Add protractor-production to config-file-paths
apb7 Jul 19, 2018
60a731c
Add copyright notice
apb7 Jul 19, 2018
dd93d40
Modify production conf
apb7 Jul 19, 2018
0bff650
Try out linux platform
apb7 Jul 19, 2018
1c09375
Change platform to mac
apb7 Jul 19, 2018
404e8d4
Merge branch 'develop' of https://github.com/oppia/oppia into browser…
apb7 Jul 20, 2018
f07b6bb
Revert Travis changes
apb7 Jul 20, 2018
8b4bc4b
Add sample env file for browserstack
apb7 Jul 20, 2018
9c7d3b4
Add configuration file for browserstack
apb7 Jul 20, 2018
14f5577
Other minor changes
apb7 Jul 20, 2018
16e0cd6
Add env example to config files
apb7 Jul 20, 2018
1ac8185
Add explanation for the process
apb7 Jul 20, 2018
bfb72ea
Add isMobile variable to existing conf file
apb7 Jul 23, 2018
0a6f6f5
Increase timeout for mobile device
apb7 Jul 23, 2018
ffb648d
Remove unused until variable
apb7 Jul 23, 2018
008326b
Add more options to example environment file
apb7 Jul 25, 2018
a3c48a9
Refactor learnerDashboard
apb7 Jul 25, 2018
40eeb8b
Add subscriptions end-to-end tests
apb7 Jul 25, 2018
a6015d1
Add export for completeSignup function
apb7 Jul 25, 2018
4bf14cc
Add reload functions in AdminPage object
apb7 Jul 25, 2018
d406e0d
Add helper functions for mobile in CreatorDashboardPage object
apb7 Jul 25, 2018
3835b02
Add demo test exploration for mobile and exploration id in feconf.py
apb7 Jul 25, 2018
88f0632
Make findExploration and findCollection functions suitable for mobile…
apb7 Jul 25, 2018
4093284
Add new spec and browser.isMobile
apb7 Jul 25, 2018
b79707b
Add end-to-end test suite for mobile
apb7 Jul 25, 2018
4630487
Review changes and spec addition
apb7 Jul 25, 2018
3e4cebe
Add protractor mobile test tags in various html files
apb7 Jul 25, 2018
97e9bd1
Remove trailing space in top_navigation_bar_directive.html
apb7 Jul 25, 2018
8217df9
Fix findCollection
apb7 Jul 25, 2018
09a2950
Add test exploration for rating mechanism
apb7 Jul 25, 2018
d4bc754
Refactor editorAndPlayer
apb7 Jul 25, 2018
18094e9
Add tests for ratings
apb7 Jul 25, 2018
f816deb
Add rating spec
apb7 Jul 25, 2018
91115ef
Review changes: capitalize letters
apb7 Jul 25, 2018
e4dec5d
Add console errors to libraryFlow
apb7 Jul 25, 2018
791c4be
Accept alert for reloadCollection
apb7 Jul 25, 2018
e84ee31
Modify loginFlow suit
apb7 Jul 25, 2018
631e037
Fix protractor mobile test tag
apb7 Jul 25, 2018
81adf35
Parallelize tests in configuration file
apb7 Jul 25, 2018
f88408f
Add refactored specs to Travis
apb7 Jul 25, 2018
92d9be7
Change the word collection to exploration
apb7 Jul 25, 2018
ffa8680
Remove extra spaces in comment
apb7 Jul 25, 2018
8846971
Change line break
apb7 Jul 25, 2018
8c9596d
Add test exploration for learner flow in mobile
apb7 Jul 25, 2018
0552a47
Add test collection for learner flow
apb7 Jul 25, 2018
ebc3585
Add entry in demo collections
apb7 Jul 25, 2018
7802dbe
Modify reloadCollection function to take in the collectionId
apb7 Jul 25, 2018
28ffb55
Add more specs to learnerFlow
apb7 Jul 25, 2018
adc48a6
Remove fit and add it in spec
apb7 Jul 25, 2018
6439520
Testing it out with Travis
apb7 Jul 26, 2018
db0dd1b
Add conditions to Travis
apb7 Jul 26, 2018
c7b854f
Minor tweak
apb7 Jul 26, 2018
57825a9
Remove demo condition in .travis.yml
apb7 Jul 27, 2018
169c0f6
Some review changes
apb7 Jul 27, 2018
24801e8
Add protractor_desktop dir
apb7 Jul 28, 2018
0ea9141
Restructure protractor dir
apb7 Jul 28, 2018
6aabbf7
Restructure protractor_mobile dir
apb7 Jul 28, 2018
32f3410
Add additional functions and do review changes
apb7 Jul 28, 2018
8936847
Restructure conf file
apb7 Jul 28, 2018
93a44b0
Restructure browserstack conf file
apb7 Jul 28, 2018
8fcfd9d
Modify test exploration
apb7 Jul 28, 2018
fb6c144
Modify tags
apb7 Jul 28, 2018
af360dc
Modify tags
apb7 Jul 28, 2018
63a44b9
Restructure travis config
apb7 Jul 28, 2018
01375d2
Merge branch 'develop' of https://github.com/oppia/oppia into browser…
apb7 Jul 28, 2018
6e857b0
Combine console with accesibility and make other changes
apb7 Jul 28, 2018
efb6d59
Remove console test from .travis.yml
apb7 Jul 28, 2018
6e0bee7
Restructure learnerDashboard tests
apb7 Jul 28, 2018
7f25654
Logout users
apb7 Jul 28, 2018
0311d78
Add libraryFlow to full in config
apb7 Jul 28, 2018
9e99692
Change function create exploration function name
apb7 Jul 28, 2018
0967489
Move ratings to libraryFlow
apb7 Jul 28, 2018
70f9c3c
Modifying config files
apb7 Jul 28, 2018
8446790
Minor tweaks
apb7 Jul 28, 2018
cbc746c
Change login name
apb7 Jul 28, 2018
11314e0
Add check for dev mode
apb7 Jul 28, 2018
7042a76
Add comments to config files
apb7 Jul 29, 2018
8fb7d5f
Rename desktop spec and modify config file
apb7 Jul 29, 2018
43aff58
.travis.yml
apb7 Jul 29, 2018
aef0569
Abstract commom functions in learnerFlow
apb7 Jul 29, 2018
978f7e7
Merge branch 'develop' of https://github.com/oppia/oppia into browser…
apb7 Jul 29, 2018
8cb84b3
Add quotes around env vars
apb7 Jul 29, 2018
a9c1dd2
libraryFlow: nit and unification
apb7 Jul 30, 2018
b2454b3
Further unification and review changes
apb7 Aug 1, 2018
0d5b996
Lint fix: fit --> it
apb7 Aug 1, 2018
8d901de
Fix learnerFlow tests
apb7 Aug 1, 2018
909c7fd
Fix var .travis.yml
apb7 Aug 2, 2018
f0b05d5
Modify check in protractor config file
apb7 Aug 2, 2018
cd1c226
Add exploration_player_test.yaml
apb7 Aug 2, 2018
3398940
Move functions to LearnerDashboardPage and unify learnerFlow
apb7 Aug 2, 2018
fe2f70c
Further unification and removal of branching
apb7 Aug 2, 2018
03c11ce
Modify space-before-function-paren rule and fix corresponding errors
apb7 Aug 2, 2018
b7ce549
Modify html tag and learnerFlow for mobile
apb7 Aug 2, 2018
9526b61
Minor tweak
apb7 Aug 2, 2018
61164f2
Merge branch 'develop' of https://github.com/oppia/oppia into browser…
apb7 Aug 2, 2018
37fc3a3
Create submitSearchQuery function
apb7 Aug 3, 2018
6a53974
Reduce branching in libraryFlow
apb7 Aug 3, 2018
b253c5a
Merge branch 'develop' of https://github.com/oppia/oppia into browser…
apb7 Aug 3, 2018
89f31bb
Remove file
apb7 Aug 3, 2018
b42cffd
Unification and fixes
apb7 Aug 3, 2018
99e4474
Fix search service
apb7 Aug 3, 2018
ba52b39
Add necessary tags
apb7 Aug 3, 2018
8096443
Reduce branching and other minor tweaks
apb7 Aug 3, 2018
796ff4c
Rename loginFlow to profileFlow and necessary changes
apb7 Aug 3, 2018
66f71b2
Rename profileFlow to profileMenuFlow and necessary changes
apb7 Aug 3, 2018
ad88702
Add explanation for desktop part of test
apb7 Aug 3, 2018
4e14f53
Remove console error msgs
apb7 Aug 4, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add additional functions and do review changes
  • Loading branch information
apb7 committed Jul 28, 2018
commit 32f3410590ec25011f2ef5936e637c59cc23736b
4 changes: 2 additions & 2 deletions core/tests/protractor_utils/AdminPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ var AdminPage = function(){
'.protractor-test-reload-all-explorations-button'
));

var reloadCollectionButton = element.all(by.css(
var reloadCollectionButtons = element.all(by.css(
'.protractor-test-reload-collection-button'));
Copy link
Contributor

Choose a reason for hiding this comment

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

reloadCollectionButtons to reflect plurality here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!


var explorationTitleElement = function(explorationElement) {
Expand All @@ -62,7 +62,7 @@ var AdminPage = function(){
};

this.reloadCollection = function(collectionId) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe have a config that's DEV_MODE and then in this file do:

    if (DEV_MODE) {
      var reloadAllExplorationsButtons = ...;
      ...
      this.reloadCollection = ...;
      ...
    }

since these are only relevant there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I didn't think about this. How do we access this variable here?

Copy link
Contributor

@hoangviet1993 hoangviet1993 Jul 28, 2018

Choose a reason for hiding this comment

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

When --prod_env flag is used, the globals DEV_MODE variable is flipped.
1)The Activities tab in Admin Page is disable and replaced with the line :
The 'Activities' tab is not available in the production environment.

2)The little "Dev Mode" tag at the right corner of every page also disappears.

You can take advantage of these GUI changes accordingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for this -- quite helpful :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!

Copy link
Member

Choose a reason for hiding this comment

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

Ah I was thinking of just declaring it similarly to what you have with browser.isMobile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't this approach a bit better in the sense that we are actually checking for the dev mode? I tested it out and it seems to work fine although we can go the other way if you'd like :)

Copy link
Member

@seanlip seanlip Jul 29, 2018

Choose a reason for hiding this comment

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

You mean @hoangviet1993's approach? Yep, I'm fine with either; I was just answering your earlier question about "how do we access this variable here". (Sorry for not being clear.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, understood! Thanks :)

reloadCollectionButton.get(collectionId).click();
reloadCollectionButtons.get(collectionId).click();
general.acceptAlert();
// Time is needed for the reloading to complete.
waitFor.textToBePresentInElement(
Expand Down
2 changes: 1 addition & 1 deletion core/tests/protractor_utils/CreatorDashboardPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ var CreatorDashboardPage = function() {
var createExplorationButton =
element(by.css('.protractor-test-create-exploration'));
var createExplorationButtonMobile =
element(by.css('.protractor-mobile-test-create-exploration'));
element(by.css('.protractor-test-create-new-exploration-button'));

// Returns a promise of all explorations with the given name.
var _getExplorationElements = function(explorationTitle) {
Expand Down
10 changes: 5 additions & 5 deletions core/tests/protractor_utils/LibraryPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ var LibraryPage = function(){
var languageSelector = forms.MultiSelectEditor(
element(by.css('.protractor-test-search-bar-language-selector'))
);
var searchInput = element.all(
var searchInputs = element.all(
by.css('.protractor-test-search-input'));
Copy link
Contributor

Choose a reason for hiding this comment

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

searchInputs to reflect plurality.


// Returns a promise of all explorations with the given name.
Expand Down Expand Up @@ -156,9 +156,9 @@ var LibraryPage = function(){
// The second input element is visible when the library
// page is rendered for mobile device.

var desktopSearchInput = searchInput.first();
var desktopSearchInput = searchInputs.first();
Copy link
Member

Choose a reason for hiding this comment

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

Might as well define these vars in the relative branches. Split the doc above and put it in each branch:

In mobile devices, the search input ...

In desktop devices, the search input is represented in the ...

(say)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!

// get function is a zero-based index.
var mobileSearchInput = searchInput.get(1);
var mobileSearchInput = searchInputs.get(1);
if (browser.isMobile) {
Copy link
Member

Choose a reason for hiding this comment

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

Should not duplicate so much code with findCollection(). Have both methods delegate to an internal method for selectLibraryTile(tileName) perhaps.

Actually the name is ambiguous. Looks like it's just entering input in search bar, does it hit enter? Maybe submitSearchQuery() would be better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created a separate function, _submitSearchQuery(searchQuery) for this and moved the duplicate code under it and then called it appropriately. Done, thanks!

mobileSearchInput.clear();
mobileSearchInput.sendKeys(explorationTitle);
Expand All @@ -176,9 +176,9 @@ var LibraryPage = function(){
// The second input element is visible when the library
// page is rendered for mobile device.

var desktopSearchInput = searchInput.first();
var desktopSearchInput = searchInputs.first();
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks!

// get function is a zero-based index.
var mobileSearchInput = searchInput.get(1);
var mobileSearchInput = searchInputs.get(1);
if (browser.isMobile) {
mobileSearchInput.clear();
mobileSearchInput.sendKeys(collectionTitle);
Expand Down
14 changes: 13 additions & 1 deletion core/tests/protractor_utils/users.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,23 @@ var createAndLoginAdminUser = function(email, username) {
adminPage.updateRole(username, 'admin');
};

var createAdminMobile = function(email, username) {
createAndLoginAdminUserMobile(email, username);
logout();
};

var createAndLoginAdminUserMobile = function(email, username) {
login(email, true);
_completeSignup(username);
};


exports.login = login;
exports.completeSignup = _completeSignup;
exports.logout = logout;
exports.createUser = createUser;
exports.createAndLoginUser = createAndLoginUser;
exports.createModerator = createModerator;
exports.createAdmin = createAdmin;
exports.createAndLoginAdminUser = createAndLoginAdminUser;
exports.createAdminMobile = createAdminMobile;
exports.createAndLoginAdminUserMobile = createAndLoginAdminUserMobile;