Skip to content
This repository was archived by the owner on Jan 26, 2021. It is now read-only.

Add automated tests for registration and shift # page #335

Merged
merged 8 commits into from
Jun 13, 2016

Conversation

smarshy
Copy link
Contributor

@smarshy smarshy commented Jun 10, 2016

This PR is to add automated tests for admin/volunteer registration and shift # page

Earlier, 3 tests existed for name, address and city fields on admin/volunteer registration page.
Now tests for state, country phone number, organization and email have been added. The tests for address, city, state, country have been clubbed as a single location test, hence ensuring that 1 test does the job which earlier 4 tests would have completed. This has been done for both volunteer and admin separately.

Please review.

@tapasweni-pathak I have added in more tests here for the shift # page too. The list of all tests now added are

  • test for phone field (both volunteer and admin)
  • test for state field (both volunteer and admin)
  • test for email field (both volunteer and admin)
  • test for country field (both volunteer and admin)
  • test for organization field (both volunteer and admin)
  • test for retention of form field values, when reloaded (both volunteer and admin)
  • test for search events on shift # page
  • test for shift # when events are outdated
  • test for shift # when shift does not have slots left

In total there are 150 tests now and this is the corresponding Build

This PR is related to #346

@tapaswenipathak
Copy link
Contributor

@smarshy What will be the total count after this?

Can this be added to Travis CI?

self.driver.find_element_by_id('id_state').send_keys('admin-state')
self.driver.find_element_by_id('id_country').send_keys('admin-country')
self.driver.find_element_by_id('id_city').send_keys('!$@%^#&admin-city')
self.driver.find_element_by_id('id_state').send_keys('!$@%^#&admin-state')
Copy link
Contributor

Choose a reason for hiding this comment

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

@smarshy Why do we have these characters 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.

@tapasweni-pathak This is for checking regex validation/ legit characters for a field as per the model. In this part of the test, I am checking what would happen if I enter special characters in city, state and country. I should get a warning for each of these fields then that these are not valid characters and registration shouldn't be allowed

@smarshy
Copy link
Contributor Author

smarshy commented Jun 11, 2016

@tapasweni-pathak Effectively, 2 tests - 143 in total.
I added 8 tests:

  • test for phone field (both volunteer and admin)
  • test for state field (both volunteer and admin)
  • test for email field (both volunteer and admin)
  • test for country field (both volunteer and admin)

But I have combined tests for address, city (already written) with tests for state, country into single location test, so effectively the count has come down to only 2 more tests

These do have those 8 failures/1 error and no travis. But I have pushed same changes in another branch, where I check everything with travis - this is the build

It is passing, so nothing would be broken. This Pr can be merged first, and the other will add travis and remove errors later.
Else, after they are merged, I need to rebase this before it can be considered for merging.

@tapaswenipathak
Copy link
Contributor

tapaswenipathak commented Jun 12, 2016

Nice. :)

This Pr can be merged first, and the other will add travis and remove errors later.
Else, after they are merged, I need to rebase this before it can be considered for merging

@smarshy I didn't get this part. :(

In the automated testing repository we can afford to have 8 failures and 1 error. We will push all the newly created tests to the automated testing repository once before the mid term evaluation and once before the final evaluation.

There should be no failures or errors in Travis CI and in the code in our main VMS repository. So after this pull request, #334 and #332 are merged, CI will give us no errors or failures. It will be having total 141 test cases and VMS will have 143 test cases, all passing.

I am thinking of adding all the new test cases to Travis CI as well. We can do this once in two weeks or however you want to do it, I have no issues.

While contributing to some open source projects I have seen that some projects have options to run a part of test cases. They set importance to their test cases and the contributor can opt for that part. This is not encouraged but good option to provide. Our Travis anyways will have all. Do you have any idea how we can accomplish this? I have no idea. But I'll try to find this out.

@smarshy
Copy link
Contributor Author

smarshy commented Jun 12, 2016

@tapasweni-pathak Test cases in the sense run only one file/app/testcase or something like that or do you want to configure a file that for eg. runs only unit tests; user will only run that before contributing (as it was previously)

The only reason why this doesn't have the fixed tests is because I wanted to work from another branch for adding all the tests. Else everything would have got pushed into the branch of the previous prs itself. This Pr if merged later after the other two will cause build failures (if I don't rebase). What I was saying that in the end the total result would be no build failures. If they are merged first, I rebase this pr and this gets merged then no failures. Or if this is merged first, they are merged later then no failures again

@smarshy smarshy changed the title Add automated tests for registration page Add automated tests for registration and shift # page Jun 13, 2016
@tapaswenipathak
Copy link
Contributor

@smarshy All unit test cases and some automated test cases.

I can still run the unit test cases alone, isn't it?

I'll test and merge this one by 9:00 P.M today. Then once May/Rose enable CI for us, I'll merge the other two. This flow should not create any issue or extra work for you, right?

self.driver.find_element_by_id('id_unlisted_organization').send_keys('volunteer-org')
self.driver.find_element_by_xpath('//form[1]').submit()

# verify that user wasn't registered and that field values are not erased
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments. 👍

@tapaswenipathak
Copy link
Contributor

tapaswenipathak commented Jun 13, 2016

In total there are 150 tests now

@smarshy So you separated the one you combined before for address, city (already written) with tests for state, country into single location test, is it?

The build looks good. :)

@smarshy
Copy link
Contributor Author

smarshy commented Jun 13, 2016

@tapasweni-pathak no, it wouldn't create extra work. But by default, all tests would e run now on the run test command. If you just want to run unit tests, you will have to run them from each app separately. Following commands need to be run to cover all unit tests -

python manage.py test event.tests.test_services
python manage.py test shift.tests.test_services
python manage.py test volunteer.tests.test_services
python manage.py test job.tests.test_services
python manage.py test organization.tests.test_services

Total tests that should be run : 50 (9 + 22 + 8 + 7 + 4)
There are no unit tests for administrator, registration, authentication and home apps

We can add a single script to cover all the unit tests instead of typing each command

@tapaswenipathak
Copy link
Contributor

tapaswenipathak commented Jun 13, 2016

@smarshy I tested it. It said

Ran 159 tests in 2008.597s

FAILED (failures=8, errors=3)

Thanks for the work. :)

Yes, I agree with this and I know this. What I am saying is it a bit different thing. Let's discuss it in tomorrow's meeting. :)

@tapaswenipathak tapaswenipathak merged commit 12a68b4 into anitab-org:develop Jun 13, 2016
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants