Create Base module for Tests #681
Conversation
Monal5031
added
Priority: HIGH
Program: GSOC
Tests/administrator/test_working.py
Outdated
| element.clear() | ||
| element.send_keys('pycon') | ||
| element.send_keys(Keys.RETURN) | ||
| self.assertNotIn('No results found.', self.driver.page_source) |
There was a problem hiding this comment.
No new line at the end in all the files.
There was a problem hiding this comment.
In Pycharm it shows those newline in all files, otherwise pep8 integrated in pycharm and codacy must've raised the same issue. 😕
There was a problem hiding this comment.
Yes it's recommended by pep8, due to consistency in every line being terminated by '\n'.
There was a problem hiding this comment.
Yes I follow that rule, here is the screenshot of the file in pycharm which shows that there is a newline at end.
I don't know why it isn't being shown here 😕
There was a problem hiding this comment.
Can you check if some other IDE/editor shows the newline too? Something like Sublime, VSCode, etc.?
There was a problem hiding this comment.
Okay I'll check it, Thanks.
|
I'd like to merge this pull request on a priority basis, as another PR is blocked due to this. Can someone in the VMS team review or should I go ahead and merge? @ayushgarg1804 |
Tests/administrator/test_working.py
Outdated
| @@ -0,0 +1,42 @@ | |||
| # Third Party Imports | |||
There was a problem hiding this comment.
I find these comments redundant and making below imports harder to read. Are they even required?
There was a problem hiding this comment.
I can remove them, but it is a good practice to separate the type of imports namely (third-party, local-project, django)
Tests/administrator/test_working.py
Outdated
| # Django imports | ||
| from django.contrib.staticfiles.testing import LiveServerTestCase | ||
|
|
||
|
|
|
Hey you all, I'm not able to keep up to VMS PR's and issue queue, but this should never have been merged. What is the use of adding base test? In future this would further require cleanup and lowering the percentage and confusion for contributors. What does these tests test? Can anyone please link me to the use of having such test in a project like VMS. It is great to have thoughts from people having different level of understanding of Django, Tests, but PR's should be merged after getting a review from someone who is a bit more known to all these. cc @systers/maintainers-vms @systers/admins. |
|
|
|
@tapasweni-pathak This was in my gsoc proposal (infrastructure/automation for web) that I'll start by creating a separate dir for Tests where in only tests corresponding to project will be maintained not a different repo itself. @tonythomas01 since I'm relocating tests so the original command to run test wont work we have to give the new path where they are located and I have documented this in the next PR like how to run tests for this new structure. |
|
tbh if this move is not critical to your GSoC project, I would recommend leaving it where it is though (and inside apps/tests/), rather than ending up in some issues later. |
|
@tonythomas01 @tapasweni-pathak Its not much needed change so I've reverted this change in my selenium integration PR. |
Description
This PR aims to create a base module for Tests in root of project where all the tests will be written.
Fixes #674
Type of Change:
Code/Quality Assurance Only
How Has This Been Tested?
Run the following command in root of project
Checklist:
Delete irrelevant options.
Code/Quality Assurance Only