Skip to content
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

unit test for webhook #808

Closed
zhan9san opened this issue Jul 8, 2021 · 6 comments · Fixed by #836
Closed

unit test for webhook #808

zhan9san opened this issue Jul 8, 2021 · 6 comments · Fixed by #836

Comments

@zhan9san
Copy link
Contributor

zhan9san commented Jul 8, 2021

Hi @gonchik

I noticed you have #806 merged. Thank you.

As we just invoke the original rest api #806, the unit tests would be too simple.

Do we need to add this kind of code style of unit test for this feature?
If yes, I'd like to summit another PR.
If no, could you please show some better code style of unit test?

Here is the snippet. It seems stupid, but it does work.

from atlassian import Bitbucket
from unittest.mock import patch
from unittest import TestCase

class TestWebhook(TestCase):

    def setUp(self):
        self.fake_data = {
            "project_key": "fake_project",
            "repository_slug": "fake_repo",
            "name": "fake_name",
            "events": ["repo:refs_changed", "pr:merged", "pr:opened"],
            "webhook_url": "https://example.com",
            "active": True,
            "secret": "fake_secret",
        }

    def test_create_webhook(self):
        return_value = {
            "id": 10,
            "name": "fake_name",
            "createdDate": 1513106011000,
            "updatedDate": 1513106011000,
            "events": [
                "repo:refs_changed",
                "pr:merged",
                "pr:opened"
            ],
            "configuration": {
                "secret": "fake_secret"
            },
            "url": "https://example.com",
            "active": True
        }
        with patch.object(Bitbucket, "post", return_value=return_value):
            bitbucket = Bitbucket(url="https://bitbucket.example.com", username="username", password="password")
            rst = bitbucket.create_webhook(self.fake_data["project_key"], self.fake_data["repository_slug"],
                                           self.fake_data["name"], self.fake_data["events"],
                                           self.fake_data["webhook_url"], self.fake_data["active"])
            self.assertEqual(rst["name"], self.fake_data["name"])
            self.assertEqual(rst["events"], self.fake_data["events"])
            self.assertEqual(rst["configuration"]["secret"], self.fake_data["secret"])
            self.assertEqual(rst["url"], self.fake_data["webhook_url"])
            self.assertEqual(rst["active"], self.fake_data["active"])

@Spacetown
Copy link
Contributor

Hi @zhan9san ,
there is already a test framework. You can extend the test and add a response to the folder test/response.

@zhan9san
Copy link
Contributor Author

zhan9san commented Jul 9, 2021

Hi @Spacetown

Thanks for your attention.

I agree with you that there is already a test framework, and it does work well. I appreciate that you guys have made this great project.

But in the other hand, I find that it tests too many functions rather than one tiny bit of functionality.

I'd like to know would it be possible to introduce tiny independent unit test.

As we just wrap the original api, the unit test for the corresponding function would be simple, do you agree?

Or do we really need unit test for this kind of feature?

@Spacetown
Copy link
Contributor

Spacetown commented Jul 9, 2021

Hi @zhan9san,
for the test framework the modified response from the documnetation is used. Please do not add a second possibility to add the expected response.
Best regards, Michael

@Spacetown
Copy link
Contributor

I think this should work.

  1. Create a file atlassian-python-api/tests/responses/bitbucket/server/rest/api/1.0/projects/fake_project/repos/fake_repo/webhooks/POST with the content:
responses = {
    '{"active": True, "events": ["repo:refs_changed", "pr:merged", "pr:opened"], "webhook_url": "https://example.com", "name": fake_name}' : {
            "id": 10,
            "name": "fake_name",
            "createdDate": 1513106011000,
            "updatedDate": 1513106011000,
            "events": [
                "repo:refs_changed",
                "pr:merged",
                "pr:opened"
            ],
            "configuration": {
                "secret": "fake_secret"
            },
            "url": "https://example.com",
            "active": True
     }
}
  1. Add a new test file for the non OO implementation of BB:
from atlassian import Bitbucket

BITBUCKET = None
try:
    from .mockup import mockup_server

    BITBUCKET = Bitbucket(
        "{}/bitbucket/server".format(mockup_server()), username="username", password="password"
    )
except ImportError:
    pass

    def test_create_webhook(self):
        rst = BIiTBUCKET.create_webhook(
            "fake_project",
            "fake_repo",
            "fake_name",
            ["repo:refs_changed", "pr:merged", "pr:opened"],
            "https://example.com",
            True
        )
        self.assertEqual(rst["name"], self.fake_data["name"])
        self.assertEqual(rst["events"], self.fake_data["events"])
        self.assertEqual(rst["configuration"]["secret"], self.fake_data["secret"])
        self.assertEqual(rst["url"], self.fake_data["webhook_url"])
        self.assertEqual(rst["active"], self.fake_data["active"])

@zhan9san
Copy link
Contributor Author

zhan9san commented Aug 9, 2021

Hi @Spacetown

Thanks for your advice.

I re-considered this approach of test and found it is really amazing.

I narrowly considered the test is unit test, but it's more like an integration test here. Session.request is mocked in the furthest edge parts we cannot control. That's a good point.

The rest tests will be added these days.

Besides, this inspires me to add tests to Artifactory Project.

@Spacetown
Copy link
Contributor

@zhan9san That was the reason why I introduced this mookup. Ther where a couble of errors in refactroing the BB implementation. For implementing the OO interface I searched for a way to test all functions in the interface and implemnted this mookup. It's a little bit of work to implement the responses but than you can test a specific endpoint in detail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants