Skip to content

Commit

Permalink
Make tests more terse + fix typing on UniqueOpts (#13)
Browse files Browse the repository at this point in the history
I'm getting to a place soon where I'm going to have to add a new set of
tests for asynchronous operations, so in preparation for that, here we
go through and make the tests more terse in a few ways:

* Remove time mocks for tests they're not needed (most of them).

* Remove local variables that aren't needed (instead just use the value
  directly). i.e. `args` can just be `JobArgs()` values, `UniqueOpts`
  can just go right onto `InsertOpts`.

* Replace the name `result` with the more specific `insert_res`
  (suggesting it's a result for an insert rather than an arbitrary
  result).

Also found a bad value for `by_args` in one test and realized that it
wasn't being detected because the fields on `UniqueOpts` didn't have
type annotations on them. Add type annotations to make this impossible
in the future.
  • Loading branch information
brandur authored Jul 4, 2024
1 parent a761187 commit 51c2851
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 115 deletions.
10 changes: 5 additions & 5 deletions src/riverqueue/client.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from dataclasses import dataclass
from datetime import datetime, timezone, timedelta
from typing import Any, Optional, Protocol, Tuple, List, Callable
from typing import Any, Literal, Optional, Protocol, Tuple, List, Callable

from .driver import GetParams, JobInsertParams, DriverProtocol, ExecutorProtocol
from .model import InsertResult
Expand Down Expand Up @@ -37,10 +37,10 @@ class InsertOpts:

@dataclass
class UniqueOpts:
by_args: Optional[Any] = None
by_period: Optional[Any] = None
by_queue: Optional[Any] = None
by_state: Optional[Any] = None
by_args: Optional[Literal[True]] = None
by_period: Optional[int] = None
by_queue: Optional[Literal[True]] = None
by_state: Optional[list[str]] = None


class Client:
Expand Down
77 changes: 25 additions & 52 deletions tests/client_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,23 +37,17 @@ def client(mock_driver) -> Client:
return Client(mock_driver)


@patch("datetime.datetime")
def test_insert_with_only_args(mock_datetime, client, mock_exec):
mock_datetime.now.return_value = datetime(2024, 6, 1, 12, 0, 0, tzinfo=timezone.utc)

def test_insert_with_only_args(client, mock_exec):
mock_exec.job_get_by_kind_and_unique_properties.return_value = None
mock_exec.job_insert.return_value = "job_row"

result = client.insert(SimpleArgs())
insert_res = client.insert(SimpleArgs())

mock_exec.job_insert.assert_called_once()
assert result.job == "job_row"
assert insert_res.job == "job_row"


@patch("datetime.datetime")
def test_insert_tx(mock_datetime, mock_driver, client):
mock_datetime.now.return_value = datetime(2024, 6, 1, 12, 0, 0, tzinfo=timezone.utc)

def test_insert_tx(mock_driver, client):
mock_exec = MagicMock(spec=ExecutorProtocol)
mock_exec.job_get_by_kind_and_unique_properties.return_value = None
mock_exec.job_insert.return_value = "job_row"
Expand All @@ -66,47 +60,38 @@ def mock_unwrap_executor(tx: sqlalchemy.Transaction):

mock_driver.unwrap_executor.side_effect = mock_unwrap_executor

result = client.insert_tx(mock_tx, SimpleArgs())
insert_res = client.insert_tx(mock_tx, SimpleArgs())

mock_exec.job_insert.assert_called_once()
assert result.job == "job_row"

assert insert_res.job == "job_row"

@patch("datetime.datetime")
def test_insert_with_opts(mock_datetime, client, mock_exec):
mock_datetime.now.return_value = datetime(2024, 6, 1, 12, 0, 0, tzinfo=timezone.utc)

args = SimpleArgs()
insert_opts = InsertOpts(queue="high_priority", unique_opts=None)
def test_insert_with_opts(client, mock_exec):
insert_opts = InsertOpts(queue="high_priority")

mock_exec.job_get_by_kind_and_unique_properties.return_value = None
mock_exec.job_insert.return_value = "job_row"

result = client.insert(args, insert_opts=insert_opts)
insert_res = client.insert(SimpleArgs(), insert_opts=insert_opts)

mock_exec.job_insert.assert_called_once()
assert result.job == "job_row"
assert insert_res.job == "job_row"

# Check that the InsertOpts were correctly passed to make_insert_params
call_args = mock_exec.job_insert.call_args[0][0]
assert call_args.queue == "high_priority"


@patch("datetime.datetime")
def test_insert_with_unique_opts_by_args(mock_datetime, client, mock_exec):
mock_datetime.now.return_value = datetime(2024, 6, 1, 12, 0, 0, tzinfo=timezone.utc)

args = SimpleArgs()
unique_opts = UniqueOpts(by_args=True)
insert_opts = InsertOpts(unique_opts=unique_opts)
def test_insert_with_unique_opts_by_args(client, mock_exec):
insert_opts = InsertOpts(unique_opts=UniqueOpts(by_args=True))

mock_exec.job_get_by_kind_and_unique_properties.return_value = None
mock_exec.job_insert.return_value = "job_row"

result = client.insert(args, insert_opts=insert_opts)
insert_res = client.insert(SimpleArgs(), insert_opts=insert_opts)

mock_exec.job_insert.assert_called_once()
assert result.job == "job_row"
assert insert_res.job == "job_row"

# Check that the UniqueOpts were correctly processed
call_args = mock_exec.job_insert.call_args[0][0]
Expand All @@ -117,59 +102,47 @@ def test_insert_with_unique_opts_by_args(mock_datetime, client, mock_exec):
def test_insert_with_unique_opts_by_period(mock_datetime, client, mock_exec):
mock_datetime.now.return_value = datetime(2024, 6, 1, 12, 0, 0, tzinfo=timezone.utc)

args = SimpleArgs()
unique_opts = UniqueOpts(by_period=900)
insert_opts = InsertOpts(unique_opts=unique_opts)
insert_opts = InsertOpts(unique_opts=UniqueOpts(by_period=900))

mock_exec.job_get_by_kind_and_unique_properties.return_value = None
mock_exec.job_insert.return_value = "job_row"

result = client.insert(args, insert_opts=insert_opts)
insert_res = client.insert(SimpleArgs(), insert_opts=insert_opts)

mock_exec.job_insert.assert_called_once()
assert result.job == "job_row"
assert insert_res.job == "job_row"

# Check that the UniqueOpts were correctly processed
call_args = mock_exec.job_insert.call_args[0][0]
assert call_args.kind == "simple"


@patch("datetime.datetime")
def test_insert_with_unique_opts_by_queue(mock_datetime, client, mock_exec):
mock_datetime.now.return_value = datetime(2024, 6, 1, 12, 0, 0, tzinfo=timezone.utc)

args = SimpleArgs()
unique_opts = UniqueOpts(by_queue=True)
insert_opts = InsertOpts(unique_opts=unique_opts)
def test_insert_with_unique_opts_by_queue(client, mock_exec):
insert_opts = InsertOpts(unique_opts=UniqueOpts(by_queue=True))

mock_exec.job_get_by_kind_and_unique_properties.return_value = None
mock_exec.job_insert.return_value = "job_row"

result = client.insert(args, insert_opts=insert_opts)
insert_res = client.insert(SimpleArgs(), insert_opts=insert_opts)

mock_exec.job_insert.assert_called_once()
assert result.job == "job_row"
assert insert_res.job == "job_row"

# Check that the UniqueOpts were correctly processed
call_args = mock_exec.job_insert.call_args[0][0]
assert call_args.kind == "simple"


@patch("datetime.datetime")
def test_insert_with_unique_opts_by_state(mock_datetime, client, mock_exec):
mock_datetime.now.return_value = datetime(2024, 6, 1, 12, 0, 0, tzinfo=timezone.utc)

args = SimpleArgs()
unique_opts = UniqueOpts(by_state=["available", "running"])
insert_opts = InsertOpts(unique_opts=unique_opts)
def test_insert_with_unique_opts_by_state(client, mock_exec):
insert_opts = InsertOpts(unique_opts=UniqueOpts(by_state=["available", "running"]))

mock_exec.job_get_by_kind_and_unique_properties.return_value = None
mock_exec.job_insert.return_value = "job_row"

result = client.insert(args, insert_opts=insert_opts)
insert_res = client.insert(SimpleArgs(), insert_opts=insert_opts)

mock_exec.job_insert.assert_called_once()
assert result.job == "job_row"
assert insert_res.job == "job_row"

# Check that the UniqueOpts were correctly processed
call_args = mock_exec.job_insert.call_args[0][0]
Expand Down
90 changes: 32 additions & 58 deletions tests/driver/riversqlalchemy/sqlalchemy_driver_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,27 +25,21 @@ def client(driver: riversqlalchemy.Driver) -> Client:
return Client(driver)


@patch("datetime.datetime")
def test_insert_with_only_args(mock_datetime, client, driver):
mock_datetime.now.return_value = datetime(2024, 6, 1, 12, 0, 0, tzinfo=timezone.utc)
args = SimpleArgs()
result = client.insert(args)
assert result.job
def test_insert_with_only_args(client, driver):
insert_res = client.insert(SimpleArgs())
assert insert_res.job


@patch("datetime.datetime")
def test_insert_tx(mock_datetime, client, driver, engine):
mock_datetime.now.return_value = datetime(2024, 6, 1, 12, 0, 0, tzinfo=timezone.utc)

def test_insert_tx(client, driver, engine):
with engine.begin() as tx:
args = SimpleArgs()
result = client.insert_tx(tx, args)
assert result.job
insert_res = client.insert_tx(tx, args)
assert insert_res.job

job = driver.unwrap_executor(tx).job_get_by_kind_and_unique_properties(
GetParams(kind=args.kind)
)
assert job == result.job
assert job == insert_res.job

with engine.begin() as tx2:
job = driver.unwrap_executor(tx2).job_get_by_kind_and_unique_properties(
Expand All @@ -56,61 +50,41 @@ def test_insert_tx(mock_datetime, client, driver, engine):
tx.rollback()


@patch("datetime.datetime")
def test_insert_with_opts(mock_datetime, client, driver):
mock_datetime.now.return_value = datetime(2024, 6, 1, 12, 0, 0, tzinfo=timezone.utc)
args = SimpleArgs()
def test_insert_with_opts(client, driver):
insert_opts = InsertOpts(queue="high_priority", unique_opts=None)
result = client.insert(args, insert_opts=insert_opts)
assert result.job

insert_res = client.insert(SimpleArgs(), insert_opts=insert_opts)
assert insert_res.job

@patch("datetime.datetime")
def test_insert_with_unique_opts_by_args(mock_datetime, client, driver):
mock_datetime.now.return_value = datetime(2024, 6, 1, 12, 0, 0, tzinfo=timezone.utc)

args = SimpleArgs()
unique_opts = UniqueOpts(by_args=args)
insert_opts = InsertOpts(unique_opts=unique_opts)
result = client.insert(args, insert_opts=insert_opts)
assert result.job
result2 = client.insert(args, insert_opts=insert_opts)
assert result.job == result2.job
def test_insert_with_unique_opts_by_args(client, driver):
insert_opts = InsertOpts(unique_opts=UniqueOpts(by_args=True))
insert_res = client.insert(SimpleArgs(), insert_opts=insert_opts)
assert insert_res.job
insert_res2 = client.insert(SimpleArgs(), insert_opts=insert_opts)
assert insert_res.job == insert_res2.job


@patch("datetime.datetime")
def test_insert_with_unique_opts_by_period(mock_datetime, client, driver):
mock_datetime.now.return_value = datetime(2024, 6, 1, 12, 0, 0, tzinfo=timezone.utc)

args = SimpleArgs()
unique_opts = UniqueOpts(by_period=900)
insert_opts = InsertOpts(unique_opts=unique_opts)
result = client.insert(args, insert_opts=insert_opts)
result2 = client.insert(args, insert_opts=insert_opts)
assert result.job == result2.job


@patch("datetime.datetime")
def test_insert_with_unique_opts_by_queue(mock_datetime, client, driver):
mock_datetime.now.return_value = datetime(2024, 6, 1, 12, 0, 0, tzinfo=timezone.utc)
insert_opts = InsertOpts(unique_opts=UniqueOpts(by_period=900))
insert_res = client.insert(SimpleArgs(), insert_opts=insert_opts)
insert_res2 = client.insert(SimpleArgs(), insert_opts=insert_opts)
assert insert_res.job == insert_res2.job

args = SimpleArgs()
unique_opts = UniqueOpts(by_queue=True)
insert_opts = InsertOpts(unique_opts=unique_opts)
result = client.insert(args, insert_opts=insert_opts)
assert result.job
result2 = client.insert(args, insert_opts=insert_opts)
assert result.job == result2.job

def test_insert_with_unique_opts_by_queue(client, driver):
insert_opts = InsertOpts(unique_opts=UniqueOpts(by_queue=True))
insert_res = client.insert(SimpleArgs(), insert_opts=insert_opts)
assert insert_res.job
insert_res2 = client.insert(SimpleArgs(), insert_opts=insert_opts)
assert insert_res.job == insert_res2.job

@patch("datetime.datetime")
def test_insert_with_unique_opts_by_state(mock_datetime, client, driver):
mock_datetime.now.return_value = datetime(2024, 6, 1, 12, 0, 0, tzinfo=timezone.utc)

args = SimpleArgs()
unique_opts = UniqueOpts(by_state=["available", "running"])
insert_opts = InsertOpts(unique_opts=unique_opts)
result = client.insert(args, insert_opts=insert_opts)
assert result.job
result2 = client.insert(args, insert_opts=insert_opts)
assert result.job == result2.job
def test_insert_with_unique_opts_by_state(client, driver):
insert_opts = InsertOpts(unique_opts=UniqueOpts(by_state=["available", "running"]))
insert_res = client.insert(SimpleArgs(), insert_opts=insert_opts)
assert insert_res.job
insert_res2 = client.insert(SimpleArgs(), insert_opts=insert_opts)
assert insert_res.job == insert_res2.job

0 comments on commit 51c2851

Please sign in to comment.