Test to cover JIRA token and password authentication. #424
Conversation
|
@KevinMGranger I was wondering if we should actually catch the JIRAError and then tell that this may be as well wrong api key instead of just passing original JIRAError with: Note that I have created test pelorus atlassian.net that is used in the connection tests, they require valid token (even if it's not active) to create api key to communicate with JIRA instance and I am using it in test. |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
mpryc
force-pushed
the
issue_423
branch
2 times, most recently
from
78668be
to
f8d2fdf
Compare
Added test which checks for the deprecated password authentication. JIRA uses same method to authenticate in the cloud jira instance for both deprecated password authentication and token based auth. This may lead to the situation where wrong API key is actually considered password and then failure exporter is presenting deprecation error. This change also moves connection func to separate private one for easy testing and readibility + may in the future include more sophisticated auth methods. Added proper headers to the files which were modified.
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
There was a problem hiding this comment.
I've got some small notes, which I've modified locally and can push if you wish.
Overall:
- What does each test do? Can we document them?
- JIRA failure exporter should not use basic auth with username/password #423 says the deprecation should be documented. So we either need documentation in this PR or to not mark it as resolved with this PR.
- I'm confused by a deprecation raising an exception -- shouldn't it let you keep going but warn you?
- If we're switching to insisting on an email address, should we change the envionment variable as well?
| def mock_search_issues(self): | ||
| return [] | ||
|
|
||
| monkeypatch = MonkeyPatch() |
There was a problem hiding this comment.
The preferred way to use monkeypatch is as a fixture-- we can just make a parameter named monkeypatch and it will work.
There was a problem hiding this comment.
Yeah, I had troubles with fixture here, but will play with it.
| # | ||
|
|
||
| import pytest | ||
| from _pytest.monkeypatch import MonkeyPatch |
There was a problem hiding this comment.
MonkeyPatch is exposed within pytest, no need to look for it in a private module.
There was a problem hiding this comment.
@KevinMGranger normally yes by fixture, but for some reason pytest was failing that the module was not found.
|
The small changes I made are here: https://github.com/kevinmgranger/pelorus/tree/issue_423 |
mpryc
added
the
compatibility
Added test which checks for the deprecated password authentication.
JIRA uses same method to authenticate in the cloud jira instance for both deprecated password authentication and token based auth.
This may lead to the situation where wrong API key is actually considered password and then failure exporter is presenting deprecation error.
This change also moves connection func to separate private one for easy testing and readibility + may in the future include more sophisticated auth methods.
Added proper headers to the files which were modified.
resolves #423
@redhat-cop/mdt