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

Add more tests for ngi_data and fix some bugs #29

Merged
merged 7 commits into from
Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 21 additions & 26 deletions daily_read/ngi_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def save_data(self):

abs_path = os.path.join(self.data_location, project_record.relative_path)
if os.path.dirname(abs_path) != source_year_dir: # This should really never happen
raise ValueError(f"Error with paths, dirname of {abs_path} should be {year_dir}")
raise ValueError(f"Error with paths, dirname of {abs_path} should be {source_year_dir}")

with open(abs_path, mode="w") as fh:
log.debug(f"Writing data for {project_record.project_id} to {abs_path}")
Expand Down Expand Up @@ -188,7 +188,9 @@ def get_modified_or_new_projects(self):
internal_id,
internal_name,
) = ProjectDataRecord.data_from_file(project_path)
project_record = ProjectDataRecord(project_path, orderer, project_dates, internal_id, internal_name)
project_record = ProjectDataRecord(
project_path, orderer, project_dates, internal_id, internal_name, self.config.STATUS_PRIORITY_REV
)
projects_list.append(project_record)

return projects_list
Expand Down Expand Up @@ -218,23 +220,7 @@ class ProjectDataRecord(object):
Raises ValueError if orderer is not present in data, if data is given
"""

dates_prio = {
"All Raw data Delivered": 5,
"All Samples Sequenced": 4,
"Library QC finished": 3,
"Reception Control finished": 2,
"Samples Received": 1,
"None": 0,
}

def __init__(
self,
relative_path,
orderer,
project_dates,
internal_id=None,
internal_name=None,
):
def __init__(self, relative_path, orderer, project_dates, internal_id=None, internal_name=None, dates_prio=None):
"""relative_path: e.g. "NGIS/2023/NGI0002313.json" """
node_year, file_name = os.path.split(relative_path)
node, year = os.path.split(node_year)
Expand Down Expand Up @@ -268,7 +254,7 @@ def __init__(

# If multiple statuses for the same date, choose the one with highest prio
if len(latest_statuses) > 1:
self.status = sorted(latest_statuses, key=lambda s: self.dates_prio[s], reverse=True)[0]
self.status = sorted(latest_statuses, key=lambda s: dates_prio[s], reverse=True)[0]
else:
self.status = latest_statuses[0]
else:
Expand Down Expand Up @@ -326,6 +312,7 @@ def __init__(self, config):
self.name = "NGI Stockholm"
self.dirname = "NGIS"
self.statusdb_session = statusdb.StatusDBSession(config)
self.dates_prio = config.STATUS_PRIORITY_REV

def get_data(self, project_id=None, close_date=None):
"""Fetch data from Stockholm StatusDB.
Expand All @@ -350,7 +337,7 @@ def get_data(self, project_id=None, close_date=None):
internal_name = row.value["project_name"]

self.data[portal_id] = ProjectDataRecord(
relative_path, orderer, project_dates, internal_id, internal_name
relative_path, orderer, project_dates, internal_id, internal_name, self.dates_prio
)

return self.data
Expand All @@ -364,7 +351,15 @@ def get_entry(self, project_id):
order_year = row.value["order_year"]
portal_id = row.value["portal_id"]
relative_path = f"{self.dirname}/{order_year}/{portal_id}.json"
self.data[portal_id] = ProjectDataRecord(relative_path, data=row.value)

project_dates = row.value["proj_dates"]
orderer = row.value["orderer"]
internal_id = row.value["project_id"]
internal_name = row.value["project_name"]

self.data[portal_id] = ProjectDataRecord(
relative_path, orderer, project_dates, internal_id, internal_name, self.dates_prio
)
return
raise ValueError(f"Project {project_id} not found in statusdb")

Expand All @@ -376,8 +371,8 @@ def __init__(self, config):
self.name = "SNP&SEQ"
self.dirname = "SNPSEQ"

def get_data(self, project_id=None):
pass
def get_data(self, project_id=None, close_date=None):
return {}


class UGCProjectData(object):
Expand All @@ -387,5 +382,5 @@ def __init__(self, config):
self.name = "Uppsala Genome Center"
self.dirname = "UGC"

def get_data(self, project_id=None):
pass
def get_data(self, project_id=None, close_date=None):
return {}
2 changes: 1 addition & 1 deletion tests/.test.env
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ DAILY_READ_REPORTS_LOCATION='tests/test_reports_location'
DAILY_READ_FETCH_FROM_NGIS = true
DAILY_READ_FETCH_FROM_SNPSEQ = true
DAILY_READ_FETCH_FROM_UGC = true
#DAILY_READ_LOG_LOCATION='tests/log_location'
DAILY_READ_USERS_LIST_LOCATION=
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, replaced a comment with a likely typo? Shouldn't there be something on the right side of the equal sign?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah no, its not a typo. The field can be empty and should be empty if you want DailyRead to look at all users. If its not, the reports will only be generated for users in the list.

58 changes: 52 additions & 6 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import copy
import couchdb
import git
import json
import os
import pytest
import json
from daily_read import ngi_data

from datetime import date, timedelta
import copy

from daily_read import ngi_data, config

dummy_order_open = {
"orderer": "[email protected]",
Expand All @@ -25,8 +27,8 @@
"2023-07-28": ["All Samples Sequenced"],
"2023-07-29": ["All Raw data Delivered"],
},
"internal_id": "P123456",
"internal_name": "D.Dummysson_23_01",
"internal_id": "P123455",
"internal_name": "D.Dummysson_23_02",
}

order_portal_resp_order_processing = {
Expand Down Expand Up @@ -159,7 +161,7 @@
"fields": {
"assigned_node": "Stockholm",
"project_ngi_identifier": "P123455",
"project_ngi_name": "D.Dummysson_23_01",
"project_ngi_name": "D.Dummysson_23_02",
},
}

Expand Down Expand Up @@ -298,13 +300,15 @@ def data_repo_full(
@pytest.fixture
def mock_project_data_record():
def _method(status):
config_values = config.Config()
if status == "open":
mock_record = ngi_data.ProjectDataRecord(
"NGIS/2023/NGI123456.json",
dummy_order_open["orderer"],
dummy_order_open["project_dates"],
dummy_order_open["internal_id"],
dummy_order_open["internal_name"],
config_values.STATUS_PRIORITY_REV,
)
if status == "closed":
mock_record = ngi_data.ProjectDataRecord(
Expand All @@ -313,6 +317,7 @@ def _method(status):
dummy_order_closed["project_dates"],
dummy_order_closed["internal_id"],
dummy_order_closed["internal_name"],
config_values.STATUS_PRIORITY_REV,
)
if status == "open_with_report":
mock_record = ngi_data.ProjectDataRecord(
Expand All @@ -321,6 +326,7 @@ def _method(status):
dummy_order_open["project_dates"],
dummy_order_open["internal_id"],
dummy_order_open["internal_name"],
config_values.STATUS_PRIORITY_REV,
)
return mock_record

Expand Down Expand Up @@ -357,3 +363,43 @@ def json(self):
)

return MockResponse(None, 404)


@pytest.fixture
def mocked_statusdb_conn_rows():
"""To substitute return value for the daily_read.statusdb.StatusDBSession.rows method"""
row1 = couchdb.client.Row(
id="b77d4f",
key=["XXXX-XX-XX", "P123457"],
value={
"orderer": "[email protected]",
"portal_id": "NGI123457",
"order_year": "2023",
"project_id": "P123457",
"project_name": "D.Dummysson_23_03",
"proj_dates": {
"2023-06-15": ["Samples Received"],
"2023-06-28": ["Reception Control finished", "Library QC finished"],
},
"status": "Ongoing",
},
)
row2 = couchdb.client.Row(
id="b77d4g",
key=[(date.today() - timedelta(days=31)).strftime("%Y-%m-%d"), "P123458"],
value={
"orderer": "[email protected]",
"portal_id": "NGI123458",
"order_year": "2023",
"project_id": "P123458",
"project_name": "T.Dummysson_23_04",
"proj_dates": {
"2023-06-15": ["Samples Received"],
"2023-06-28": ["Reception Control finished", "Library QC finished"],
"2023-07-28": ["All Samples Sequenced"],
"2023-07-29": ["All Raw data Delivered"],
},
"status": "Closed",
},
)
return [row1, row2]
6 changes: 3 additions & 3 deletions tests/test_daily_report.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import dotenv
import datetime
import dotenv
import os
from unittest import mock
from conftest import mocked_requests_get

from conftest import mocked_requests_get
from unittest import mock

from daily_read import daily_report, config, ngi_data, order_portal

Expand Down
110 changes: 106 additions & 4 deletions tests/test_ngi_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

import dotenv
from unittest import mock
import pytest
import logging

from daily_read import ngi_data, config

dotenv.load_dotenv()

LOGGER = logging.getLogger(__name__)

####################################################### TESTS #########################################################

Expand Down Expand Up @@ -129,12 +132,113 @@ def test_modified_or_new_tracked(data_repo_tracked):
assert len(set(file_names)) == 0


def test_get_unique_orderers(data_repo_full):
"""Test getting unique orders in the project data from statusdb"""
config_values = config.Config()
with mock.patch("daily_read.statusdb.StatusDBSession"):
data_master = ngi_data.ProjectDataMaster(config_values)
data_master.get_data()
orderers = data_master.find_unique_orderers()
assert orderers == set(["[email protected]"])


def test_user_list(data_repo_full, tmp_path, mocked_statusdb_conn_rows):
"""Test getting and reading users from the user list url"""
config_values = config.Config()
temp_file = tmp_path / "test_file.txt"
temp_file.write_text("[email protected]\[email protected]")
config_values.USERS_LIST_LOCATION = temp_file
with mock.patch("daily_read.statusdb.StatusDBSession"):
data_master = ngi_data.ProjectDataMaster(config_values)
assert not set(["[email protected]", "[email protected]"]) ^ set(data_master.user_list)

data_master.sources[0].statusdb_session.rows.return_value = mocked_statusdb_conn_rows
data_master.get_data()
data_master.save_data()
orderers = data_master.find_unique_orderers()
assert orderers == set(["[email protected]", "[email protected]"])


def test_save_data_to_disk(data_repo_full, mocked_statusdb_conn_rows):
"""Test saving in git repo the data gotten from statusdb"""
config_values = config.Config()
with mock.patch("daily_read.statusdb.StatusDBSession"):
data_master = ngi_data.ProjectDataMaster(config_values)
data_master.sources[0].statusdb_session.rows.return_value = mocked_statusdb_conn_rows
data_master.get_data()
data_master.save_data()
assert os.path.exists(os.path.join(config_values.DATA_LOCATION, "NGIS/2023/NGI123457.json"))
assert os.path.exists(os.path.join(config_values.DATA_LOCATION, "NGIS/2023/NGI123458.json"))


def test_get_data_with_project(data_repo_full, mocked_statusdb_conn_rows):
"""Test getting data for a specific order"""
config_values = config.Config()
with mock.patch("daily_read.statusdb.StatusDBSession"):
data_master = ngi_data.ProjectDataMaster(config_values)
data_master.sources[0].statusdb_session.rows.return_value = mocked_statusdb_conn_rows
data_master.get_data("NGI123457")
assert len(data_master.data.keys()) == 1
assert "NGI123457" in data_master.data


def test_get_data_with_project_unknown(data_repo_full, mocked_statusdb_conn_rows):
"""Test error thrown when the order specified is not found in statusdb"""
config_values = config.Config()
with mock.patch("daily_read.statusdb.StatusDBSession"):
data_master = ngi_data.ProjectDataMaster(config_values)
data_master.sources[0].statusdb_session.rows.return_value = mocked_statusdb_conn_rows
with pytest.raises(ValueError, match="Project NGI123 not found in statusdb") as err:
data_master.get_data("NGI123")


@mock.patch("daily_read.statusdb.StatusDBSession")
def test_data_loc_not_abs(mock_status):
"""Test error thrown when given data location is not an absolute path"""
config_values = config.Config()
config_values.DATA_LOCATION = "tests/test_data_location"
with pytest.raises(
ValueError, match=f"Data location is not an absolute path: {config_values.DATA_LOCATION}"
) as err:
ngi_data.ProjectDataMaster(config_values)


@mock.patch("daily_read.statusdb.StatusDBSession")
def test_data_loc_not_dir(mock_status, tmp_path):
"""Test error thrown when data location is not a directory"""
config_values = config.Config()
temp_file = tmp_path / "test_file.txt"
temp_file.write_text("test")
config_values.DATA_LOCATION = temp_file
with pytest.raises(
ValueError, match=f"Data Location exists but is not a directory: {config_values.DATA_LOCATION}"
) as err:
ngi_data.ProjectDataMaster(config_values)


def test_get_data_with_no_project_dates(data_repo_full, mocked_statusdb_conn_rows, caplog):
"""Test log output when no project dates are found in statusdb for a specifi project"""
from copy import deepcopy

config_values = config.Config()
with mock.patch("daily_read.statusdb.StatusDBSession"):
data_master = ngi_data.ProjectDataMaster(config_values)
data_master.sources[0].statusdb_session.rows.return_value = mocked_statusdb_conn_rows
proj_no_dates = deepcopy(data_master.sources[0].statusdb_session.rows.return_value[0])
proj_no_dates.value["proj_dates"] = {}
proj_no_dates.value["portal_id"] = "NGI123459"
data_master.sources[0].statusdb_session.rows.return_value.append(proj_no_dates)
with caplog.at_level(logging.INFO):
data_master.get_data("NGI123459")
assert len(data_master.data.keys()) == 1
assert "NGI123459" in data_master.data
assert "No project dates found for NGI123459" in caplog.text


# Planned tests #


def test_getting_data():
# Test getting data
# When everything is ok
# When some source fails
# When all sources fails
# When some source is not enabled
Expand All @@ -145,8 +249,6 @@ def test_getting_data():
pass


# Test saving data

# Test add data for staging
# If other things are added already
# Test adding something that isn't changed
Expand Down
Loading