-
-
Notifications
You must be signed in to change notification settings - Fork 643
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
Add integration testing using appium for the iOS #602
Conversation
Automated message from Dropbox CLA bot @kunall17, it looks like you've already signed the Dropbox CLA. Thanks! |
Cooool! |
int-test/readme.md
Outdated
@@ -0,0 +1,47 @@ | |||
## Installation for the appium testing dependencies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe move the file to docs/appium.md
int-test/tests/login.py
Outdated
@@ -0,0 +1,62 @@ | |||
from appium import webdriver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we want to use Node for test writing?
As the project itself is JavaScript based and is not guaranteed that contributors even know Python or have it installed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True that,
Thoughts @timabbott @neerajwahi ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the libraries are just as good, doing JavaScript for the mobile tests is a great strategy, since the fewer languages one needs to learn, the better.
Already RN has JavaScript/iOS/Android, that's enough things to know :)
int-test/readme.md
Outdated
|
||
To install appium execute from the root folder of zulip-mobile | ||
``` | ||
cd int-test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might want to add a script to that (maybe later, is good as instructions for now)
Pushed in javascript, old one in here https://github.com/kunall17/zulip-mobile/tree/int-test-python |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the hard work on this @kunall17 ! Some comments below
__int-tests__/basic.js
Outdated
|
||
const options = { | ||
validEmail: 'iago@zulip.com', | ||
inValidEmail: 'invalid@zulip.com', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: inValid -> invalid
__int-tests__/basic.js
Outdated
await driver.refresh(); | ||
}); | ||
|
||
it('Test to connect to the URL', () => driver |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I would reword the test names as "Successfully connects to a Zulip realm" and "Signs in using the dev auth backend"
__int-tests__/basic.js
Outdated
}); | ||
|
||
it('Test to connect to the URL', () => driver | ||
.sleep(2000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sleep
could result in the test suite being very flaky. We should instead wait for the particular elements we need to appear
package.json
Outdated
@@ -17,7 +17,8 @@ | |||
"test:flow": "flow", | |||
"test:unit": "jest", | |||
"test:coveralls": "jest --coverage && cat ./coverage/lcov.info | coveralls", | |||
"test": "npm run test:lint && npm run test:coveralls && npm run test:flow" | |||
"test": "npm run test:lint && npm run test:coveralls && npm run test:flow", | |||
"test-int": "mocha --compilers js:babel-core/register __int-tests__/basic.js" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I'd probably rename 'test-int' to something a bit more descriptive like 'test-appium'
__int-tests__/setup.js
Outdated
@@ -0,0 +1,11 @@ | |||
/*eslint-disable */ | |||
|
|||
var wd = require("wd"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rewrite this in ES6 style?
package.json
Outdated
"babel-eslint": "^7.1.1", | ||
"babel-jest": "^20.0.0", | ||
"babel-polyfill": "^6.23.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does babel-polyfill do?
src/start/AuthScreen.js
Outdated
@@ -33,7 +33,7 @@ class AuthScreen extends React.Component { | |||
<View style={styles.container}> | |||
<Label | |||
style={[styles.field]} | |||
value={this.props.realm} | |||
text={this.props.realm} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the change from value
to text
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one was a bug, but it is actually fixed in master. Needs rebase I guess?
.eslintrc
Outdated
@@ -45,7 +45,7 @@ | |||
"react/no-array-index-key": 0, | |||
"react/no-unused-prop-types": 0, | |||
"import/prefer-default-export": 0, | |||
"import/no-extraneous-dependencies": ["error", {"devDependencies": ["**/__tests__/*.js"]}], | |||
"import/no-extraneous-dependencies": ["error", {"devDependencies": ["**/__tests__/*.js", "**/__int-tests__/*.js"]}], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I'd rename the __int-test__
directory to __appium-tests__
or even just __tests__
(and let the fact that it's a top-level directory indicates that it's integration and not unit tests)
Updated works on jest! :) |
That's a good start to our end-to-end testing. |
Currently test's for connection with server and login through dev auth backend is implemented
http://chase-seibert.github.io/blog/2017/01/06/appium-react-native-quickstart.html is a great guide to get started with the appium testing