-
Notifications
You must be signed in to change notification settings - Fork 6
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
Include more test cases to check issue ticket operations #39
Conversation
Signed-off-by: Akashdeep Dhar <[email protected]>
@pytest.mark.parametrize( | ||
"srce, dest, pkey, gkey, fusr, tusr, qant, stat, rslt", | ||
[ | ||
pytest.param( |
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 see that it's using some environment variables. Is this actually connecting to something or are those just dummy values?
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.
It is connecting to both production Pagure and production GitLab. See #32 (comment).
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 it would be better to use VCR for replaying the API requests instead of trying to connect every time the test is ran. We are already using it in Anitya.
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.
Interesting. I'd definitely love to use something that helps to keep the tests definitive and remove the dependence on third-party services. Mind opening up an issue ticket to propose that?
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.
Opened #42
With da87d82, the overall coverage has now reached 100%. Please see https://github.com/gridhead/protop2g/actions/runs/6536049340/job/17746865516?pr=39#step:6:155. |
test/test_unit_tkts.py
Outdated
"user": { | ||
"full_url": "https://pagure.io/user/t0xic0der", | ||
"fullname": "Akashdeep Dhar", | ||
"name": "t0xic0der", | ||
"url_path": "user/t0xic0der", |
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 is all good, and the tests looks good! just as a suggestion to not use your own but a test account to test these stuff, because in future if there is some more people writing on test can make sure they use a single account for testing purposes hope this makes sense
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 @samyak-jn. This makes 100% sense.
I have made the suggested changes here 815c16e.
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.
Just one comment as a suggestion, otherwise lgtm!
Thanks 🚀
Signed-off-by: Akashdeep Dhar <[email protected]>
Increase the default timeout value for the requests from 30 seconds to 60 seconds Signed-off-by: Akashdeep Dhar <[email protected]>
Signed-off-by: Akashdeep Dhar <[email protected]>
Include more test cases to check issue ticket operations
Fixes #38
This brings the total test coverage to around 92%.