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

Added scripts for ci infrastructure #420

Open
wants to merge 5 commits into
base: develop
from

Conversation

4 participants
@Fenn-CS
Copy link
Contributor

Fenn-CS commented Jun 20, 2018

Description

Added scripts that will be supplied to jenkins for test jobs

Fixes #418

Type of Change:

  • Code
  • Quality Assurance
  • Documentation

How Has This Been Tested?

Tested locally

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have made corresponding changes to the documentation

@Fenn-CS Fenn-CS requested review from MehaKaushik , nida and MayBurgos Jun 20, 2018

@Fenn-CS Fenn-CS force-pushed the Fenn-CS:add-ci-infrastructure branch from d484dca to f92ea9a Jun 20, 2018

@nida

This comment has been minimized.

Copy link
Member

nida commented Jun 20, 2018

What has failed?

@Fenn-CS

This comment has been minimized.

Copy link
Contributor

Fenn-CS commented Jun 20, 2018

@nida it's flake8 I just need to fix formatting

@Fenn-CS Fenn-CS force-pushed the Fenn-CS:add-ci-infrastructure branch from febd92d to e5da443 Jun 20, 2018

added ci scripts and test runner
Signed-off-by: Fenn-25 <fenn25.fn@gmail.com>

@Fenn-CS Fenn-CS force-pushed the Fenn-CS:add-ci-infrastructure branch from e5da443 to 5f43573 Jun 20, 2018

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jun 20, 2018

Coverage Status

Coverage decreased (-0.8%) to 97.855% when pulling 06712ea on Fenn-CS:add-ci-infrastructure into 2943851 on systers:develop.

argv = []
if self.verbosity == 0:
argv.append('--quiet')
if self.verbosity == 2:

This comment has been minimized.

@nida

nida Jun 21, 2018

Member

Out of curiosity, what would verbosity=1 mean?

This comment has been minimized.

@Fenn-CS

Fenn-CS Jun 22, 2018

Contributor

Hello @nida by default the value is one.
https://github.com/Fenn-CS/portal/blob/5f43573a75e81075906f83b17df76ef6b887f34a/systers_portal/systers_portal/settings/runner.py#L4

And on one, all the "verbose" reports irrespective will be displayed on shell.

This comment has been minimized.

@nida

nida Jun 26, 2018

Member

Makes sense. Thanks for the explanation

changed django test runner to support pytest
Signed-off-by: Fenn-25 <fenn25.fn@gmail.com>

@Fenn-CS Fenn-CS force-pushed the Fenn-CS:add-ci-infrastructure branch from 8eb6e3d to f91cc0a Jun 21, 2018

Fenn-CS added some commits Jun 22, 2018

added implicit wait to allow selenium tests pass
Signed-off-by: Fenn-25 <fenn25.fn@gmail.com>
changed url path for pytest runner
Signed-off-by: Fenn-25 <fenn25.fn@gmail.com>
@nida
Copy link
Member

nida left a comment

Requesting a few comments that i had overlooked. Previously, Please address


#Run tests, $1, $2, $3 ... allow for command line argurments

python systers_portal/manage.py test --settings=systers_portal.settings.testing $1 $2 $3 $4 $5

This comment has been minimized.

@nida

nida Jun 26, 2018

Member

What are these parameters $1 you provide description in the comment.

@@ -28,7 +28,7 @@ class SeleniumTestCase(StaticLiveServerTestCase):
def setUpClass(cls):
super().setUpClass()
cls.browser = browsers['firefox']()
cls.browser.implicitly_wait(0)
cls.browser.implicitly_wait(30)

This comment has been minimized.

@nida

nida Jun 26, 2018

Member

Can you add a comment about why the time is set to 30s

This comment has been minimized.

@Fenn-CS

Fenn-CS Jun 26, 2018

Contributor

This will permit the driver wait for a maximum of 30 seconds incase it can't find an element. Pytest seems to be too fast with switch the tests hence, selenium tests will fail if we leave it at zero initially it was 10 but I switched it to zero since tests where passing in that way.

This is ok because 30 is the maximum, this means true is returned as soon as an element is found (which is normally) the amount of the time we where supposed to wait for the page to load up.

This comment has been minimized.

@nida

nida Jun 27, 2018

Member

Comment in the file.

argv = []
if self.verbosity == 0:
argv.append('--quiet')
if self.verbosity == 2:

This comment has been minimized.

@nida

nida Jun 26, 2018

Member

Makes sense. Thanks for the explanation

added documentation for Jenkins setup
Signed-off-by: Fenn-25 <fenn25.fn@gmail.com>
@Fenn-CS

This comment has been minimized.

Copy link
Contributor

Fenn-CS commented Jul 10, 2018

This PR is not in progress and has not been abandoned either, some parts of it (Jenkins scripts) have to be removed while the changes regarding the pytest runner maintained.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment