Repository: opsdroid/opsdroid (language: python)

Issue:
support providing an access token in the configuration for the matrix connector rather than the password
This would alleviate the need to plain text the password in the config, and the access token can be deactivated. This would be a case of detecting the key in the config and skipping the login call
Improve test machinery and rewrite GitHub connector tests
# Description

This is a big one sorry folks! I started rewriting the GitHub connector tests and anded up building a whole `opsdroid.testing` submodule to help make tests nicer.

## `opsdroid.testing`
Created `opsdroid.testing` submodule with fixtures and utilities for testing. Top level fixtures are now imported from here by `conftest.py` so they can also be used by others. Also contains the following utilities.

### `ExternalAPIMockServer`

Created `ExternalAPIMockServer` utility for mocking external APIs (see https://www.youtube.com/watch?v=MDI1X6B2Ya8). The general idea with this class is to allow you to push expected responses onto a stack for each API call you expect your test to make. Then as your tests make those calls each response is popped from the stack.

Here is some example usage from the docs.

```python
import pytest
import aiohttp

from opsdroid.testing import ExternalAPIMockServer

@pytest.mark.asyncio
async def test_example():
    # Construct the mock API server and push on a test method
    mock_api = ExternalAPIMockServer()
    mock_api.add_response("/test", "GET", None, 200)

    # Create a closure function. We will have our mock_api run this concurrently with the
    # web server later.
    async def test_closure():
        # Make an HTTP request to our mock_api
        async with aiohttp.ClientSession() as session:
            async with session.get(f"{mock_api.base_url}/test") as resp:

                # Assert that it gives the expected responses
                assert resp.status == 200
                assert mock_api.called("/test")

        # Return True to we can ensure the closure was run
        return True

    # Run the test closure. This will start the web server, and once it is running await
    # the closure. Then when the closure finishes the server will be stopped again and the
    # return from the closure will be passed back.
    assert await mock_api.run_test(test)
```

### `run_unit_test`

Added a helper which runs opsdroid and a test closure together at the same time, then shuts down and cleans up opsdroid after the test finishes.

Example from the docs

```python
import pytest
from opsdroid.testing import (
    opsdroid,
    run_unit_test,
    MINIMAL_CONFIG
    )

@pytest.mark.asyncio
async def test_example(opsdroid):
    # Using the opsdrid fixture we load it with the
    # minimal example config
    await opsdroid.load(config=MINIMAL_CONFIG)

    # Check that opsdroid is not currently running
    assert not opsdroid.is_running()

    # Define an awaitable closure which asserts that
    # opsdroid is now running
    async def test():
        assert opsdroid.is_running()
        return True  # So that we can assert our run test

    # Run our closure against opsdroid. This will start opsdroid,
    # await our closure and then stop opsdroid again.
    assert await run_unit_test(opsdroid, test)
```

### `call_endpoint`

Utility which takes a running opsdroid and calls an API endpoint with a predefined request. Useful alongside `run_unit_test` to fake external webhook requests. The nice thing here is you don't need to know what address opsdroid is running at.

Example from the docs.

```python
import pytest
from opsdroid.testing import (
    opsdroid,
    call_endpoint,
    run_unit_test,
    MINIMAL_CONFIG
    )

@pytest.mark.asyncio
async def test_example(opsdroid):
    # Using the opsdrid fixture we load it with the
    # minimal example config
    await opsdroid.load(config=MINIMAL_CONFIG)

    async def test():
        # Call our endpoint by just passing
        # opsdroid, the endpoint and the method
        resp = await call_endpoint(opsdroid, "/stats", "GET")

        # Make assertions that opsdroid responded successfully
        assert resp.status == 200
        return True

    assert await run_unit_test(opsdroid, test)
```

## Core refactors

I've refactored some of the core to make things more testable. Notably I've split `load` and `unload` unto `load`, `start`, `stop` and `unload`. I've also broken some functionality out into functions like `register_skill` to give better entrypoints in the tests.

## GitHub Connector test rewrite

I've rewritten all of the GitHub connector tests using these new tools and refactors to make them more readable and also less brittle. 

Take this example. 

- We mock out the API auth
- Write a skill inline that is full of assertions
- Register the skill with opsdroid
- Write a test closure which hits the connector pretending to be a GitHub webhook
- Run everything through together

```python
@pytest.mark.asyncio
async def test_receive_issue(opsdroid, connector, mock_api):
    """Test a PR create event creates a message and parses it."""
    mock_api.add_response(
        "/user", "GET", get_response_path("github_user.json"), status=200
    )

    @match_event(Message)
    async def test_skill(opsdroid, config, event):
        assert event.connector.name == "github"
        assert event.text == "test"
        assert event.target == "opsdroid/opsdroid#740"

    opsdroid.register_skill(test_skill, config={"name": "test"})

    async def test():
        resp = await call_endpoint(
            opsdroid,
            "/connector/github",
            "POST",
            data=get_webhook_payload("github_issue_payload.json"),
        )
        assert resp.status == 201

    await mock_api.run_test(run_unit_test, opsdroid, test)
```

The result of this last line means the Mock API server will be started. Then opsdroid will be started which will call the mock API. Then our test closure will be run which will hit the connector and trigger opsdroid to run the skill, the skill will assert things and then everything will clean up.

# Checklist:

- [x] I have performed a self-review of my own code
- [x] I have made corresponding changes to the documentation (if applicable)
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes

Use pytest marks to add responses to the mock_api fixture
# Description

This solves the pytest puzzle of how to use `ExternalAPIMockServer` in the most pytest way possible.
Use pytest marks to add responses to the mock_api fixture
# Description

This solves the pytest puzzle of how to use `ExternalAPIMockServer` in the most pytest way possible.


Discussion / hints:
Could you create a PR with the changes that have been merged into your fork? Seems like a nice thing to add 😄 
There is / was one
Erm... yeah you are right! Ignore me 😬
Would be good to get some progress on that. Looks like the change is done, just needs tests and docs.
I am going to write tests for it and docs. Will create a PR later today hopefully 😄 
Just submitted a PR. Tox showed that the coverage was 100% hopefully it will work and we can merge the PR 😄 
# [Codecov](https://codecov.io/gh/opsdroid/opsdroid/pull/1558?src=pr&el=h1) Report
> Merging [#1558](https://codecov.io/gh/opsdroid/opsdroid/pull/1558?src=pr&el=desc) (76fb7f4) into [master](https://codecov.io/gh/opsdroid/opsdroid/commit/a99dfb607c4ade51a0bc6c943f1d5b85c84b4f2b?el=desc) (a99dfb6) will **increase** coverage by `0.14%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/opsdroid/opsdroid/pull/1558/graphs/tree.svg?width=650&height=150&src=pr&token=9qJfZL4bOO)](https://codecov.io/gh/opsdroid/opsdroid/pull/1558?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1558      +/-   ##
==========================================
+ Coverage   99.39%   99.54%   +0.14%     
==========================================
  Files          61       71      +10     
  Lines        3986     4162     +176     
==========================================
+ Hits         3962     4143     +181     
+ Misses         24       19       -5     
```


| [Impacted Files](https://codecov.io/gh/opsdroid/opsdroid/pull/1558?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [opsdroid/configuration/validation.py](https://codecov.io/gh/opsdroid/opsdroid/pull/1558/diff?src=pr&el=tree#diff-b3BzZHJvaWQvY29uZmlndXJhdGlvbi92YWxpZGF0aW9uLnB5) | `100.00% <ø> (ø)` | |
| [opsdroid/loader.py](https://codecov.io/gh/opsdroid/opsdroid/pull/1558/diff?src=pr&el=tree#diff-b3BzZHJvaWQvbG9hZGVyLnB5) | `100.00% <ø> (ø)` | |
| [...d/testing/mockmodules/connectors/connector\_bare.py](https://codecov.io/gh/opsdroid/opsdroid/pull/1558/diff?src=pr&el=tree#diff-b3BzZHJvaWQvdGVzdGluZy9tb2NrbW9kdWxlcy9jb25uZWN0b3JzL2Nvbm5lY3Rvcl9iYXJlLnB5) | `100.00% <ø> (ø)` | |
| [...testing/mockmodules/connectors/connector\_mocked.py](https://codecov.io/gh/opsdroid/opsdroid/pull/1558/diff?src=pr&el=tree#diff-b3BzZHJvaWQvdGVzdGluZy9tb2NrbW9kdWxlcy9jb25uZWN0b3JzL2Nvbm5lY3Rvcl9tb2NrZWQucHk=) | `100.00% <ø> (ø)` | |
| [opsdroid/testing/mockmodules/databases/database.py](https://codecov.io/gh/opsdroid/opsdroid/pull/1558/diff?src=pr&el=tree#diff-b3BzZHJvaWQvdGVzdGluZy9tb2NrbW9kdWxlcy9kYXRhYmFzZXMvZGF0YWJhc2UucHk=) | `100.00% <ø> (ø)` | |
| [...esting/mockmodules/skills/schema\_skill/\_\_init\_\_.py](https://codecov.io/gh/opsdroid/opsdroid/pull/1558/diff?src=pr&el=tree#diff-b3BzZHJvaWQvdGVzdGluZy9tb2NrbW9kdWxlcy9za2lsbHMvc2NoZW1hX3NraWxsL19faW5pdF9fLnB5) | `0.00% <ø> (ø)` | |
| [...ing/mockmodules/skills/skill/skilltest/\_\_init\_\_.py](https://codecov.io/gh/opsdroid/opsdroid/pull/1558/diff?src=pr&el=tree#diff-b3BzZHJvaWQvdGVzdGluZy9tb2NrbW9kdWxlcy9za2lsbHMvc2tpbGwvc2tpbGx0ZXN0L19faW5pdF9fLnB5) | `88.88% <ø> (ø)` | |
| [opsdroid/testing/mockmodules/skills/temp\_skill.py](https://codecov.io/gh/opsdroid/opsdroid/pull/1558/diff?src=pr&el=tree#diff-b3BzZHJvaWQvdGVzdGluZy9tb2NrbW9kdWxlcy9za2lsbHMvdGVtcF9za2lsbC5weQ==) | `0.00% <ø> (ø)` | |
| [opsdroid/connector/github/\_\_init\_\_.py](https://codecov.io/gh/opsdroid/opsdroid/pull/1558/diff?src=pr&el=tree#diff-b3BzZHJvaWQvY29ubmVjdG9yL2dpdGh1Yi9fX2luaXRfXy5weQ==) | `100.00% <100.00%> (ø)` | |
| [opsdroid/testing/\_\_init\_\_.py](https://codecov.io/gh/opsdroid/opsdroid/pull/1558/diff?src=pr&el=tree#diff-b3BzZHJvaWQvdGVzdGluZy9fX2luaXRfXy5weQ==) | `100.00% <100.00%> (ø)` | |
| ... and [20 more](https://codecov.io/gh/opsdroid/opsdroid/pull/1558/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/opsdroid/opsdroid/pull/1558?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/opsdroid/opsdroid/pull/1558?src=pr&el=footer). Last update [a99dfb6...76fb7f4](https://codecov.io/gh/opsdroid/opsdroid/pull/1558?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

@FabioRosado I've only added GET and POST because that's all I needed for my tests. Happy to add the rest at some point.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Bad stale bot!
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Please add `sphinx-autodoc-typehints` to the `extras_requires.docs` in `setup.cfg`. That should fix the docs build.
https://github.com/opsdroid/opsdroid/blob/7f036c3199a99628c0efb4d6ef142ab8f5d9a3c1/setup.cfg#L128-L133
Triggered CI
Thanks so much for picking this up @cognifloyd! Do you want to do the honour of merging once CI passes (don't worry about the coverage drop here).
OK. In my final review, I realized that I missed updating the mock API server's start method like I did with web.start. So, I added that fix including a unit test for that. Once I got that unit test passing, I also marked the mock broken skill so it gets skipped in code coverage reports.

Now, even codecov.io should be happy. As soon as all tests are green, I'll merge :smiley: 



Fix patch:
diff --git a/opsdroid/connector/matrix/connector.py b/opsdroid/connector/matrix/connector.py
index 2b68cb06a..ae4252f40 100644
--- a/opsdroid/connector/matrix/connector.py
+++ b/opsdroid/connector/matrix/connector.py
@@ -13,7 +13,7 @@
 
 from opsdroid import const, events
 from opsdroid.connector import Connector, register_event
-from voluptuous import Required
+from voluptuous import Required, Inclusive
 
 from . import events as matrixevents
 from .create_events import MatrixEventCreator
@@ -21,8 +21,9 @@
 
 _LOGGER = logging.getLogger(__name__)
 CONFIG_SCHEMA = {
-    Required("mxid"): str,
-    Required("password"): str,
+    Inclusive("mxid", "login"): str,
+    Inclusive("password", "login"): str,
+    "access_token": str,
     Required("rooms"): dict,
     "homeserver": str,
     "nick": str,
@@ -90,10 +91,11 @@ def __init__(self, config, opsdroid=None):  # noqa: D107
         self.rooms = self._process_rooms_dict(config["rooms"])
         self.room_ids = {}
         self.default_target = self.rooms["main"]["alias"]
-        self.mxid = config["mxid"]
+        self.mxid = config.get("mxid")
+        self.password = config.get("password")
+        self.access_token = config.get("access_token")
         self.nick = config.get("nick")
         self.homeserver = config.get("homeserver", "https://matrix.org")
-        self.password = config["password"]
         self.room_specific_nicks = config.get("room_specific_nicks", False)
         self.send_m_notice = config.get("send_m_notice", False)
         self.session = None
@@ -144,18 +146,14 @@ def filter_json(self):
                 "event_format": "client",
                 "account_data": {"limit": 0, "types": []},
                 "presence": {"limit": 0, "types": []},
-                "room": {
-                    "account_data": {"types": []},
-                    "ephemeral": {"types": []},
-                    "state": {"types": []},
-                },
+                "room": {"account_data": {"types": []}, "ephemeral": {"types": []}},
             }
         )
 
     async def make_filter(self, api, fjson):
         """Make a filter on the server for future syncs."""
         path = f"/_matrix/client/r0/user/{self.mxid}/filter"
-        headers = {"Authorization": f"Bearer {api.token}"}
+        headers = {"Authorization": f"Bearer {api.access_token}"}
         resp = await api.send(method="post", path=path, data=fjson, headers=headers)
 
         resp_json = await resp.json()
@@ -194,7 +192,7 @@ async def connect(self):
             pickle_key="",
             store_name="opsdroid.db" if self._allow_encryption else "",
         )
-        mapi = nio.AsyncClient(
+        self.connection = nio.AsyncClient(
             self.homeserver,
             self.mxid,
             config=config,
@@ -202,20 +200,41 @@ async def connect(self):
             device_id=self.device_id,
         )
 
-        login_response = await mapi.login(
-            password=self.password, device_name=self.device_name
-        )
-        if isinstance(login_response, nio.LoginError):
-            _LOGGER.error(
-                f"Error while connecting: {login_response.message} (status code {login_response.status_code})"
+        if self.access_token is not None:
+            self.connection.access_token = self.access_token
+
+            whoami_response = await self.connection.whoami()
+            if isinstance(whoami_response, nio.responses.WhoamiError):
+                _LOGGER.error(
+                    f"Error while connecting: {whoami_response.message} (status code {whoami_response.status_code})"
+                )
+                return
+
+            self.mxid = whoami_response.user_id
+            self.connection.user_id = self.mxid
+
+        elif self.mxid is not None and self.password is not None:
+            login_response = await self.connection.login(
+                password=self.password, device_name=self.device_name
             )
-            return
+            if isinstance(login_response, nio.LoginError):
+                _LOGGER.error(
+                    f"Error while connecting: {login_response.message} (status code {login_response.status_code})"
+                )
+                return
 
-        mapi.token = login_response.access_token
-        mapi.sync_token = None
+            self.access_token = (
+                self.connection.access_token
+            ) = login_response.access_token
+        else:
+            raise ValueError(
+                "Configuration for the matrix connector should specify mxid and password or access_token."
+            )  # pragma: no cover
+
+        self.connection.sync_token = None
 
         for roomname, room in self.rooms.items():
-            response = await mapi.join(room["alias"])
+            response = await self.connection.join(room["alias"])
             if isinstance(response, nio.JoinError):
                 _LOGGER.error(
                     f"Error while joining room: {room['alias']}, Message: {response.message} (status code {response.status_code})"
@@ -224,12 +243,10 @@ async def connect(self):
             else:
                 self.room_ids[roomname] = response.room_id
 
-        self.connection = mapi
-
         # Create a filter now, saves time on each later sync
-        self.filter_id = await self.make_filter(mapi, self.filter_json)
+        self.filter_id = await self.make_filter(self.connection, self.filter_json)
         first_filter_id = await self.make_filter(
-            mapi, '{ "room": { "timeline" : { "limit" : 1 } } }'
+            self.connection, '{ "room": { "timeline" : { "limit" : 1 } } }'
         )
 
         # Do initial sync so we don't get old messages later.
@@ -249,8 +266,20 @@ async def connect(self):
 
         if self.nick:
             display_name = await self.connection.get_displayname(self.mxid)
+            if isinstance(display_name, nio.ErrorResponse):
+                _LOGGER.warning(
+                    f"Error fetching current display_name: {display_name.message} (status code {display_name.status_code})"
+                )
+                display_name = None
+            else:
+                display_name = display_name.displayname
+
             if display_name != self.nick:
-                await self.connection.set_displayname(self.nick)
+                display_name_resp = await self.connection.set_displayname(self.nick)
+                if isinstance(display_name_resp, nio.ErrorResponse):
+                    _LOGGER.warning(
+                        f"Error setting display_name: {display_name_resp.message} (status code {display_name_resp.status_code})"
+                    )
 
     async def disconnect(self):
         """Close the matrix session."""
diff --git a/opsdroid/core.py b/opsdroid/core.py
index 0f3fca319..fb2036cc0 100644
--- a/opsdroid/core.py
+++ b/opsdroid/core.py
@@ -149,7 +149,7 @@ def handle_async_exception(loop, context):
             # pylint: disable=broad-except
             except Exception:  # pragma: nocover
                 print("Caught exception")
-                print(context)
+        print(context)
 
     def is_running(self):
         """Check whether opsdroid is running."""
diff --git a/setup.cfg b/setup.cfg
index 888e737db..7eb8099ed 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -143,6 +143,11 @@ addopts = -v
 mock_use_standalone_module = true
 markers =
     add_response: adds a response to the ExternalAPIMockServer
+    matrix_connector_config: Data to setup a matrix connector
+filterwarnings =
+    ignore:"@coroutine" decorator is deprecated since Python 3.8, use "async def" instead:DeprecationWarning
+    ignore:the imp module is deprecated in favour of importlib:DeprecationWarning
+
 
 [flake8]
 max-line-length = 80


Test patch:
diff --git a/opsdroid/connector/github/tests/test_connector_github.py b/opsdroid/connector/github/tests/test_connector_github.py
index eab021f5a..f334c89fe 100644
--- a/opsdroid/connector/github/tests/test_connector_github.py
+++ b/opsdroid/connector/github/tests/test_connector_github.py
@@ -57,7 +57,7 @@ async def test_connect(connector, mock_api):
     assert mock_api.called("/user")
     assert mock_api.call_count("/user") == 1
 
-    request = mock_api.get_request("/user")
+    request = mock_api.get_request("/user", "GET")
     assert "Authorization" in request.headers
     assert "abc123" in request.headers["Authorization"]
 
@@ -96,29 +96,20 @@ async def test_listen(connector):
 async def test_send(opsdroid, connector, mock_api):
     await opsdroid.send(
         Message(
-            text="test",
-            user="jacobtomlinson",
-            target=ISSUE_TARGET,
-            connector=connector,
+            text="test", user="jacobtomlinson", target=ISSUE_TARGET, connector=connector
         )
     )
     assert mock_api.called(COMMENTS_URI)
 
 
 @pytest.mark.add_response(
-    COMMENTS_URI,
-    "POST",
-    get_response_path("github_send_failure.json"),
-    status=400,
+    COMMENTS_URI, "POST", get_response_path("github_send_failure.json"), status=400
 )
 @pytest.mark.asyncio
 async def test_send_failure(opsdroid, connector, mock_api, caplog):
     await opsdroid.send(
         Message(
-            text="test",
-            user="jacobtomlinson",
-            target=ISSUE_TARGET,
-            connector=connector,
+            text="test", user="jacobtomlinson", target=ISSUE_TARGET, connector=connector
         )
     )
     assert mock_api.called(COMMENTS_URI)
@@ -132,10 +123,7 @@ async def test_do_not_send_to_self(opsdroid, connector, mock_api):
 
     await opsdroid.send(
         Message(
-            text="test",
-            user="opsdroid-bot",
-            target=ISSUE_TARGET,
-            connector=connector,
+            text="test", user="opsdroid-bot", target=ISSUE_TARGET, connector=connector
         )
     )
     assert not mock_api.called(COMMENTS_URI)
diff --git a/opsdroid/connector/matrix/tests/conftest.py b/opsdroid/connector/matrix/tests/conftest.py
new file mode 100644
index 000000000..505eaee84
--- /dev/null
+++ b/opsdroid/connector/matrix/tests/conftest.py
@@ -0,0 +1,150 @@
+import json
+from pathlib import Path
+
+import pytest
+
+from opsdroid.connector.matrix import ConnectorMatrix
+
+################################################################################
+# Connector and config fixtures.
+################################################################################
+
+
+@pytest.fixture
+def default_config(mock_api_obj):
+    return {"homeserver": mock_api_obj.base_url, "rooms": {"main": "#test:localhost"}}
+
+
+@pytest.fixture
+def login_config(mock_api_obj):
+    return {
+        "mxid": "@opsdroid:localhost",
+        "password": "supersecret",
+        "homeserver": mock_api_obj.base_url,
+        "rooms": {"main": "#test:localhost"},
+    }
+
+
+@pytest.fixture
+def token_config(mock_api_obj):
+    return {
+        "access_token": "token",
+        "homeserver": mock_api_obj.base_url,
+        "rooms": {"main": "#test:localhost"},
+    }
+
+
+@pytest.fixture
+async def connector(opsdroid, request, mock_api_obj):
+    if hasattr(request, "param"):
+        fix_name = request.param
+    else:
+        marker = request.node.get_closest_marker("matrix_connector_config")
+        fix_name = marker.args[0]
+
+    if isinstance(fix_name, str):
+        config = request.getfixturevalue(fix_name)
+    elif isinstance(fix_name, dict):
+        config = fix_name
+    else:
+        raise TypeError(
+            "Config should be a string name for a fixture or a dict for the config"
+        )
+
+    if "homeserver" not in config:
+        config["homeserver"] = mock_api_obj.base_url
+
+    conn = ConnectorMatrix(config, opsdroid=opsdroid)
+    yield conn
+    if conn.connection is not None:
+        await conn.disconnect()
+
+
+################################################################################
+# Sync Factory
+################################################################################
+
+
+def get_matrix_response(name):
+    return Path(__file__).parent / "responses" / f"{name}.json"
+
+
+def empty_sync(room_id):
+    with open(get_matrix_response("empty_sync_response")) as fobj:
+        empty_sync = json.load(fobj)
+
+    room_block = empty_sync["rooms"]["join"].pop("ROOMID")
+    empty_sync["rooms"]["join"][room_id] = room_block
+
+    return empty_sync
+
+
+def event_factory(event_type, content, sender):
+    return {
+        "content": content,
+        "event_id": "$1234567890987654321234567890",
+        "origin_server_ts": 9876543210,
+        "sender": sender,
+        "type": event_type,
+        "unsigned": {"age": 100},
+    }
+
+
+def message_factory(body, msgtype, sender, **extra_content):
+    content = {"msgtype": msgtype, "body": body, **extra_content}
+    return event_factory("m.room.message", content, sender)
+
+
+def sync_response(events, room_id="!12345:localhost"):
+    sync = empty_sync(room_id)
+    sync["rooms"]["join"][room_id]["timeline"]["events"] = events
+    return sync
+
+
+@pytest.fixture
+def message_sync_response(request):
+    room_id = "!12345:localhost"
+    room_id_markers = [
+        marker for marker in request.node.own_markers if marker.name == "sync_room_id"
+    ]
+    if room_id:
+        room_id = room_id_markers[0].args[0]
+
+    markers = [
+        marker
+        for marker in request.node.own_markers
+        if marker.name == "add_sync_messsage"
+    ]
+
+    events = []
+    for marker in markers:
+        events.append(message_factory(*marker.args, **marker.kwargs))
+
+    return sync_response(events, room_id=room_id)
+
+
+################################################################################
+# Response Fixtures
+################################################################################
+
+
+@pytest.fixture
+def double_filter_response(mock_api_obj):
+    mock_api_obj.add_response(
+        "/_matrix/client/r0/user/@opsdroid:localhost/filter",
+        "POST",
+        {"filter_id": "1234567890"},
+    )
+    mock_api_obj.add_response(
+        "/_matrix/client/r0/user/@opsdroid:localhost/filter",
+        "POST",
+        {"filter_id": "0987654321"},
+    )
+
+
+@pytest.fixture
+def single_message_sync_response(mock_api_obj):
+    """Return a sync response with a single test message in it."""
+    event = message_factory("Test", "m.text", "@stuart:localhost")
+    sync = sync_response([event])
+    mock_api_obj.add_response("/_matrix/client/r0/sync", "GET", sync)
diff --git a/opsdroid/connector/matrix/tests/responses/empty_sync_response.json b/opsdroid/connector/matrix/tests/responses/empty_sync_response.json
new file mode 100644
index 000000000..73d6816bd
--- /dev/null
+++ b/opsdroid/connector/matrix/tests/responses/empty_sync_response.json
@@ -0,0 +1,47 @@
+{
+    "account_data": {
+        "events": []
+    },
+    "device_lists": {
+        "changed": [],
+        "left": []
+    },
+    "device_one_time_keys_count": {
+        "signed_curve25519": 0
+    },
+    "next_batch": "s12_19_4_2_5_1_1_11_1",
+    "presence": {
+        "events": [
+        ]
+    },
+    "rooms": {
+        "invite": {},
+        "join": {
+            "ROOMID": {
+                "account_data": {
+                    "events": []
+                },
+                "ephemeral": {
+                    "events": []
+                },
+                "state": {
+                    "events": []
+                },
+                "summary": {},
+                "timeline": {
+                    "events": [],
+                    "limited": false,
+                    "prev_batch": "s11_19_4_2_5_1_1_11_1"
+                },
+                "unread_notifications": {
+                    "highlight_count": 0,
+                    "notification_count": 0
+                }
+            }
+        },
+        "leave": {}
+    },
+    "to_device": {
+        "events": []
+    }
+}
diff --git a/opsdroid/connector/matrix/tests/test_connector_connect.py b/opsdroid/connector/matrix/tests/test_connector_connect.py
new file mode 100644
index 000000000..dd39e925e
--- /dev/null
+++ b/opsdroid/connector/matrix/tests/test_connector_connect.py
@@ -0,0 +1,214 @@
+import pytest
+import logging
+
+from opsdroid.connector.matrix import ConnectorMatrix
+
+
+def test_constructor(opsdroid, default_config):
+    connector = ConnectorMatrix(default_config, opsdroid)
+    assert isinstance(connector, ConnectorMatrix)
+
+
+@pytest.fixture
+def mock_whoami_join(mock_api_obj):
+    mock_api_obj.add_response(
+        "/_matrix/client/r0/account/whoami", "GET", {"user_id": "@opsdroid:localhost"}
+    )
+    mock_api_obj.add_response(
+        "/_matrix/client/r0/join/#test:localhost",
+        "POST",
+        {"room_id": "!12355:localhost"},
+    )
+
+
+@pytest.mark.matrix_connector_config("token_config")
+@pytest.mark.asyncio
+async def test_connect_access_token(
+    opsdroid,
+    connector,
+    double_filter_response,
+    single_message_sync_response,
+    mock_whoami_join,
+    mock_api,
+):
+    assert isinstance(connector, ConnectorMatrix)
+    assert connector.access_token == "token"
+    await connector.connect()
+
+    assert mock_api.called("/_matrix/client/r0/account/whoami")
+    assert (
+        mock_api.call_count("/_matrix/client/r0/user/@opsdroid:localhost/filter") == 2
+    )
+    assert mock_api.called("/_matrix/client/r0/sync")
+    assert mock_api.called("/_matrix/client/r0/join/#test:localhost")
+
+    whoami_call = mock_api.get_request("/_matrix/client/r0/account/whoami", "GET", 0)
+    assert "access_token" in whoami_call.query
+    assert whoami_call.query["access_token"] == "token"
+
+
+@pytest.mark.matrix_connector_config("token_config")
+@pytest.mark.add_response(
+    "/_matrix/client/r0/account/whoami",
+    "GET",
+    {"errcode": "M_UNKNOWN_TOKEN", "error": "Invalid macaroon passed."},
+    status=401,
+)
+@pytest.mark.asyncio
+async def test_connect_invalid_access_token(caplog, opsdroid, connector, mock_api):
+    assert isinstance(connector, ConnectorMatrix)
+    assert connector.access_token == "token"
+    await connector.connect()
+
+    assert mock_api.called("/_matrix/client/r0/account/whoami")
+
+    assert "Invalid macaroon passed." in caplog.records[0].message
+    assert "M_UNKNOWN_TOKEN" in caplog.records[0].message
+
+
+@pytest.mark.matrix_connector_config("login_config")
+@pytest.mark.add_response(
+    "/_matrix/client/r0/login",
+    "POST",
+    {
+        "user_id": "@opsdroid:localhost",
+        "access_token": "abc123",
+        "device_id": "GHTYAJCE",
+    },
+)
+@pytest.mark.asyncio
+async def test_connect_login(
+    opsdroid,
+    connector,
+    double_filter_response,
+    single_message_sync_response,
+    mock_whoami_join,
+    mock_api,
+):
+    assert isinstance(connector, ConnectorMatrix)
+    await connector.connect()
+
+    assert mock_api.called("/_matrix/client/r0/login")
+    assert (
+        mock_api.call_count("/_matrix/client/r0/user/@opsdroid:localhost/filter") == 2
+    )
+    assert mock_api.called("/_matrix/client/r0/sync")
+    assert mock_api.called("/_matrix/client/r0/join/#test:localhost")
+
+    assert connector.access_token == connector.connection.access_token == "abc123"
+
+
+@pytest.mark.matrix_connector_config("login_config")
+@pytest.mark.add_response(
+    "/_matrix/client/r0/login",
+    "POST",
+    {"errcode": "M_FORBIDDEN", "error": "Invalid password"},
+    status=403,
+)
+@pytest.mark.asyncio
+async def test_connect_login_error(caplog, opsdroid, connector, mock_api):
+    assert isinstance(connector, ConnectorMatrix)
+    await connector.connect()
+
+    assert mock_api.called("/_matrix/client/r0/login")
+
+    assert "Invalid password" in caplog.records[0].message
+    assert "M_FORBIDDEN" in caplog.records[0].message
+
+
+@pytest.mark.matrix_connector_config("token_config")
+@pytest.mark.add_response(
+    "/_matrix/client/r0/account/whoami", "GET", {"user_id": "@opsdroid:localhost"}
+)
+@pytest.mark.add_response(
+    "/_matrix/client/r0/join/#test:localhost",
+    "POST",
+    {"errcode": "M_FORBIDDEN", "error": "You are not invited to this room."},
+    status=403,
+)
+@pytest.mark.asyncio
+async def test_connect_join_fail(
+    opsdroid,
+    connector,
+    double_filter_response,
+    single_message_sync_response,
+    mock_api,
+    caplog,
+):
+    assert isinstance(connector, ConnectorMatrix)
+    assert connector.access_token == "token"
+    await connector.connect()
+
+    assert caplog.record_tuples == [
+        (
+            "opsdroid.connector.matrix.connector",
+            logging.ERROR,
+            "Error while joining room: #test:localhost, Message: You are not invited to this room. (status code M_FORBIDDEN)",
+        )
+    ]
+
+
+@pytest.mark.matrix_connector_config(
+    {"access_token": "token", "rooms": {"main": "#test:localhost"}, "nick": "opsdroid"}
+)
+@pytest.mark.add_response(
+    "/_matrix/client/r0/profile/@opsdroid:localhost/displayname",
+    "PUT",
+    {"errcode": "M_FORBIDDEN", "error": "Invalid user"},
+    status=403,
+)
+@pytest.mark.asyncio
+async def test_connect_set_nick_errors(
+    opsdroid,
+    connector,
+    double_filter_response,
+    single_message_sync_response,
+    mock_whoami_join,
+    mock_api,
+    caplog,
+):
+    await connector.connect()
+
+    assert caplog.record_tuples == [
+        (
+            "opsdroid.connector.matrix.connector",
+            logging.WARNING,
+            "Error fetching current display_name: unknown error (status code None)",
+        ),
+        (
+            "opsdroid.connector.matrix.connector",
+            logging.WARNING,
+            "Error setting display_name: Invalid user (status code M_FORBIDDEN)",
+        ),
+    ]
+
+    caplog.clear()
+
+
+@pytest.mark.matrix_connector_config(
+    {"access_token": "token", "rooms": {"main": "#test:localhost"}, "nick": "opsdroid"}
+)
+@pytest.mark.add_response(
+    "/_matrix/client/r0/profile/@opsdroid:localhost/displayname", "PUT", {}
+)
+@pytest.mark.add_response(
+    "/_matrix/client/r0/profile/@opsdroid:localhost/displayname",
+    "GET",
+    {"displayname": "Wibble"},
+)
+@pytest.mark.asyncio
+async def test_connect_set_nick(
+    opsdroid,
+    connector,
+    double_filter_response,
+    single_message_sync_response,
+    mock_whoami_join,
+    mock_api,
+):
+    await connector.connect()
+    assert mock_api.called(
+        "/_matrix/client/r0/profile/@opsdroid:localhost/displayname", "GET"
+    )
+    assert mock_api.called(
+        "/_matrix/client/r0/profile/@opsdroid:localhost/displayname", "PUT"
+    )
diff --git a/opsdroid/testing/external_api.py b/opsdroid/testing/external_api.py
index a7ced137d..63342f448 100644
--- a/opsdroid/testing/external_api.py
+++ b/opsdroid/testing/external_api.py
@@ -5,6 +5,7 @@
 for both opsdroid core and skills.
 """
 import asyncio
+from collections import defaultdict
 import json
 from contextlib import asynccontextmanager
 from os import PathLike
@@ -64,9 +65,9 @@ def __init__(self):
         self.site = None
         self.host = "localhost"
         self.port = 8089
-        self._calls = {}
-        self.responses = {}
-        self._payloads = {}
+        self._calls = defaultdict(list)
+        self.responses = defaultdict(list)
+        self._payloads = defaultdict(list)
         self.status = "stopped"
         self.start_timeout = 10  # seconds
 
@@ -98,15 +99,12 @@ async def stop(self) -> None:
 
     async def _handler(self, request: web.Request) -> web.Response:
         route = request.path
-        if route in self._calls:
-            self._calls[route].append(request)
-        else:
-            self._calls[route] = [request]
-        if route in self._payloads:
-            self._payloads[route].append(await request.post())
-        else:
-            self._payloads[route] = [await request.post()]
-        status, response = self.responses[route].pop(0)
+        method = request.method
+
+        self._calls[(route, method)].append(request)
+        self._payloads[route].append(await request.post())
+
+        status, response = self.responses[(route, method)].pop(0)
         return web.json_response(response, status=status)
 
     @property
@@ -128,20 +126,19 @@ def add_response(
         else:
             response = response
 
-        if route in self.responses:
-            self.responses[route].append((status, response))
-        else:
-
+        if (route, method) not in self.responses:
             if method.upper() == "GET":
                 routes = [web.get(route, self._handler)]
             elif method.upper() == "POST":
                 routes = [web.post(route, self._handler)]
+            elif method.upper() == "PUT":
+                routes = [web.put(route, self._handler)]
             else:
                 raise TypeError(f"Unsupported method {method}")
-
-            self.responses[route] = [(status, response)]
             self.app.add_routes(routes)
 
+        self.responses[(route, method)].append((status, response))
+
     @asynccontextmanager
     async def running(self) -> "ExternalAPIMockServer":
         """Start the External API server within a context manager."""
@@ -158,7 +155,7 @@ def reset(self) -> None:
         else:
             raise RuntimeError("Web server must be stopped before it can be reset.")
 
-    def called(self, route: str) -> bool:
+    def called(self, route: str, method: str = None) -> bool:
         """Route has been called.
 
         Args:
@@ -168,9 +165,12 @@ def called(self, route: str) -> bool:
             Wether or not it was called.
 
         """
-        return route in self._calls
+        if not method:
+            return route in [k[0] for k in self._calls.keys()]
 
-    def call_count(self, route: str) -> int:
+        return (route, method) in self._calls
+
+    def call_count(self, route: str, method: str = None) -> int:
         """Route has been called n times.
 
         Args:
@@ -180,9 +180,15 @@ def call_count(self, route: str) -> int:
             The number of times it was called.
 
         """
-        return len(self._calls[route])
+        if not method:
+            all_calls = [
+                len(call[1]) for call in self._calls.items() if call[0][0] == route
+            ]
+            return sum(all_calls)
+
+        return len(self._calls[(route, method)])
 
-    def get_request(self, route: str, idx: int = 0) -> web.Request:
+    def get_request(self, route: str, method: str, idx: int = 0) -> web.Request:
         """Route has been called n times.
 
         Args:
@@ -193,7 +199,7 @@ def get_request(self, route: str, idx: int = 0) -> web.Request:
             The request that was made.
 
         """
-        return self._calls[route][idx]
+        return self._calls[(route, method)][idx]
 
     def get_payload(self, route: str, idx: int = 0) -> Dict:
         """Return data payload that the route was called with.
diff --git a/opsdroid/testing/tests/test_testing.py b/opsdroid/testing/tests/test_testing.py
index 2514f9270..a7689231e 100644
--- a/opsdroid/testing/tests/test_testing.py
+++ b/opsdroid/testing/tests/test_testing.py
@@ -46,7 +46,7 @@ async def test_external_api_mock_server(session):
                 assert resp.status == 200
                 assert mock_api.called("/test")
                 assert mock_api.call_count("/test") == i + 1
-                assert mock_api.get_request("/test").path == "/test"
+                assert mock_api.get_request("/test", "GET").path == "/test"
 
         async with session.post(
             f"{mock_api.base_url}/fail", data={"hello": "world"}
diff --git a/tests/test_connector_matrix.py b/tests/test_connector_matrix.py
index 45d365450..3e8e5083e 100644
--- a/tests/test_connector_matrix.py
+++ b/tests/test_connector_matrix.py
@@ -284,171 +284,6 @@ async def test_exchange_keys(self, mocker, connector):
         assert patched_keys_query.called
         patched_keys_claim.assert_called_with(patched_get_users())
 
-    async def test_connect(self, mocker, caplog, connector):
-        with amock.patch(api_string.format("login")) as patched_login, amock.patch(
-            api_string.format("join")
-        ) as patched_join, amock.patch(
-            api_string.format("sync")
-        ) as patched_sync, amock.patch(
-            api_string.format("send")
-        ) as patched_filter, amock.patch(
-            api_string.format("get_displayname")
-        ) as patched_get_nick, amock.patch(
-            api_string.format("set_displayname")
-        ) as patch_set_nick, amock.patch(
-            api_string.format("send_to_device_messages")
-        ) as patched_send_to_device, amock.patch(
-            api_string.format("should_upload_keys")
-        ) as patched_should_upload, amock.patch(
-            api_string.format("should_query_keys")
-        ) as patched_should_query, amock.patch(
-            api_string.format("keys_upload")
-        ) as patched_keys_upload, amock.patch(
-            api_string.format("keys_query")
-        ) as patched_keys_query, amock.patch(
-            "pathlib.Path.mkdir"
-        ) as patched_mkdir, amock.patch(
-            "pathlib.Path.is_dir"
-        ) as patched_is_dir, amock.patch(
-            "aiohttp.ClientSession"
-        ) as patch_cs, OpsDroid() as _:
-
-            # Skip actually creating a client session
-            patch_cs.return_value = amock.MagicMock()
-
-            patched_login.return_value = asyncio.Future()
-            patched_login.return_value.set_result(
-                nio.LoginResponse(
-                    user_id="@opsdroid:localhost",
-                    device_id="testdevice",
-                    access_token="arbitrary string1",
-                )
-            )
-
-            patched_join.return_value = asyncio.Future()
-            patched_join.return_value.set_result(
-                nio.JoinResponse(room_id="!aroomid:localhost")
-            )
-
-            patched_sync.return_value = asyncio.Future()
-            patched_sync.return_value.set_result(
-                nio.SyncResponse(
-                    next_batch="arbitrary string2",
-                    rooms={},
-                    device_key_count={"signed_curve25519": 50},
-                    device_list={"changed": [], "left": []},
-                    to_device_events={"events": []},
-                    presence_events={"events": []},
-                )
-            )
-
-            connect_response = amock.Mock()
-            connect_response.status = 200
-            connect_response.json = amock.CoroutineMock()
-            connect_response.return_value = {"filter_id": 1}
-
-            patched_filter.return_value = asyncio.Future()
-            patched_filter.return_value.set_result(connect_response)
-
-            patch_set_nick.return_value = asyncio.Future()
-            patch_set_nick.return_value.set_result(nio.ProfileSetDisplayNameResponse())
-
-            patched_is_dir.return_value = False
-            patched_mkdir.return_value = None
-
-            patched_send_to_device.return_value = asyncio.Future()
-            patched_send_to_device.return_value.set_result(None)
-
-            patched_should_upload.return_value = True
-            patched_keys_upload.return_value = asyncio.Future()
-            patched_keys_upload.return_value.set_result(None)
-
-            patched_should_query.return_value = True
-            patched_keys_query.return_value = asyncio.Future()
-            patched_keys_query.return_value.set_result(None)
-
-            await connector.connect()
-
-            if nio.crypto.ENCRYPTION_ENABLED:
-                assert patched_mkdir.called
-
-            assert patched_send_to_device.called
-            assert patched_keys_upload.called
-            assert patched_keys_query.called
-
-            assert "!aroomid:localhost" in connector.room_ids.values()
-
-            assert connector.connection.token == "arbitrary string1"
-
-            assert connector.connection.sync_token == "arbitrary string2"
-
-            connector.nick = "Rabbit Hole"
-
-            patched_get_nick.return_value = asyncio.Future()
-            patched_get_nick.return_value.set_result("Rabbit Hole")
-
-            await connector.connect()
-
-            assert patched_get_nick.called
-            assert not patch_set_nick.called
-
-            patched_get_nick.return_value = asyncio.Future()
-            patched_get_nick.return_value.set_result("Neo")
-
-            connector.mxid = "@morpheus:matrix.org"
-
-            await connector.connect()
-
-            assert patched_get_nick.called
-            patch_set_nick.assert_called_once_with("Rabbit Hole")
-
-            error_message = "Some error message"
-            error_code = 400
-
-            # test sync error
-            patched_sync.return_value = asyncio.Future()
-            patched_sync.return_value.set_result(
-                nio.SyncError(message=error_message, status_code=error_code)
-            )
-            caplog.clear()
-            await connector.connect()
-            assert [
-                f"Error during initial sync: {error_message} (status code {error_code})"
-            ] == [rec.message for rec in caplog.records]
-
-            # test join error
-            patched_sync.return_value = asyncio.Future()
-            patched_sync.return_value.set_result(
-                nio.SyncResponse(
-                    next_batch="arbitrary string2",
-                    rooms={},
-                    device_key_count={"signed_curve25519": 50},
-                    device_list={"changed": [], "left": []},
-                    to_device_events={"events": []},
-                    presence_events={"events": []},
-                )
-            )
-            patched_join.return_value = asyncio.Future()
-            patched_join.return_value.set_result(
-                nio.JoinError(message=error_message, status_code=error_code)
-            )
-            caplog.clear()
-            await connector.connect()
-            assert [
-                f"Error while joining room: {connector.rooms['main']['alias']}, Message: {error_message} (status code {error_code})"
-            ] == [rec.message for rec in caplog.records]
-
-            # test login error
-            patched_login.return_value = asyncio.Future()
-            patched_login.return_value.set_result(
-                nio.LoginError(message=error_message, status_code=error_code)
-            )
-            caplog.clear()
-            await connector.connect()
-            assert [
-                f"Error while connecting: {error_message} (status code {error_code})"
-            ] == [rec.message for rec in caplog.records]
-
     async def test_parse_sync_response(self, connector):
         connector.room_ids = {"main": "!aroomid:localhost"}
         connector.filter_id = "arbitrary string"