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

Make tests robust and independent #543

Merged
merged 28 commits into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
2102c83
Use request.node.name for make_dantzig scenarios
glatterf42 Mar 12, 2024
c625e12
Fix typo in docstring
glatterf42 Mar 12, 2024
31d7a44
Group tutorials that depend on each other
glatterf42 Mar 12, 2024
d68f757
Give every platform a test-specific name
glatterf42 Mar 12, 2024
6a8bd54
Use test-specific name for scenarios
glatterf42 Mar 12, 2024
c7d94ef
Use test-specific name for platforms
glatterf42 Mar 12, 2024
38ac174
Revert temporary fix from #510
glatterf42 Mar 12, 2024
b955aab
Copy model["dantzig"] properly
glatterf42 Mar 12, 2024
3b62932
Avoid searching for to-be-created test-specific platforms
glatterf42 Mar 18, 2024
540d613
Search for correct new scenario name
glatterf42 Mar 18, 2024
4127f57
Increase default cell runtime on GHA
glatterf42 Mar 18, 2024
6c111d8
Use more test-specific platforms
glatterf42 Mar 18, 2024
9f6afa3
Make call to to-be-tested function explicit
glatterf42 Mar 18, 2024
4a67769
Rename expected data files for individual tests
glatterf42 Mar 18, 2024
f3615ce
Call Scenario deletion function explicitly
glatterf42 Mar 22, 2024
6216c02
Call TimeSeries deletion function explicitly
glatterf42 Mar 22, 2024
12bfcc3
Add type hint for correct specific backend type
glatterf42 Mar 22, 2024
12464ed
Remove timeseries from jindex explicitly
glatterf42 Mar 22, 2024
d469fd3
Discard tmp_path directories after each test
glatterf42 Sep 11, 2024
bb48e78
Drop workaround for jupyter/nbclient#85
glatterf42 Sep 11, 2024
918298f
Make second R tutorial independent with helper function
glatterf42 Sep 11, 2024
34f0595
Mark first tutorial tests as flaky on Windows
glatterf42 Sep 12, 2024
6a50d32
Mark test_del_ts as flaky on Windows
glatterf42 Sep 12, 2024
5b30a63
Remove xfail marker for consistently xpassing test
glatterf42 Sep 12, 2024
fc76fab
Distinguish platforms for more stability
glatterf42 Sep 12, 2024
61b40cb
Expand typing in .testing.data
khaeru Sep 12, 2024
97f1121
Remove editor-specific notebook metadata
glatterf42 Sep 12, 2024
749709e
Address review comments
glatterf42 Sep 12, 2024
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
3 changes: 1 addition & 2 deletions .github/workflows/pytest.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -135,13 +135,12 @@ jobs:
shell: Rscript {0}

- name: Run test suite using pytest
# FIXME: Use --numprocesses=auto once flaky tests are fixed
run: |
pytest ixmp \
-m "not performance" \
--color=yes -rA --verbose \
--cov-report=xml \
--numprocesses=1
--numprocesses=auto --dist=loadgroup
shell: bash

- name: Upload test coverage to Codecov.io
Expand Down
1 change: 1 addition & 0 deletions ixmp/backend/jdbc.py
Original file line number Diff line number Diff line change
Expand Up @@ -728,6 +728,7 @@ def del_ts(self, ts):

# Aggressively free memory
self.gc()
self.jindex.pop(ts, None)

def check_out(self, ts, timeseries_only):
with _handle_jexception():
Expand Down
1 change: 1 addition & 0 deletions ixmp/core/platform.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ def export_timeseries_data(
- subannual
- year
- value

default : bool, optional
:obj:`True` to include only TimeSeries versions marked as default.
model: str, optional
Expand Down
24 changes: 19 additions & 5 deletions ixmp/testing/data.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
# Methods are in alphabetical order
from itertools import product
from math import ceil
from typing import Any, List
from typing import Any, List, Optional

import genno
import numpy as np
import pandas as pd
import pint
import pytest

from ixmp import Platform, Scenario, TimeSeries
from ixmp.backend import IAMC_IDX
Expand Down Expand Up @@ -158,7 +159,12 @@ def add_test_data(scen: Scenario):
return t, t_foo, t_bar, x


def make_dantzig(mp: Platform, solve: bool = False, quiet: bool = False) -> Scenario:
def make_dantzig(
mp: Platform,
solve: bool = False,
quiet: bool = False,
request: Optional["pytest.FixtureRequest"] = None,
) -> Scenario:
"""Return :class:`ixmp.Scenario` of Dantzig's canning/transport problem.

Parameters
Expand All @@ -169,6 +175,8 @@ def make_dantzig(mp: Platform, solve: bool = False, quiet: bool = False) -> Scen
If :obj:`True`. then solve the scenario before returning. Default :obj:`False`.
quiet : bool, optional
If :obj:`True`, suppress console output when solving.
request : :class:`pytest.FixtureRequest`, optional
If present, use for a distinct scenario name for each test.

Returns
-------
Expand All @@ -189,14 +197,20 @@ def make_dantzig(mp: Platform, solve: bool = False, quiet: bool = False) -> Scen
# Initialize a new Scenario, and use the DantzigModel class' initialize()
# method to populate it
annot = "Dantzig's transportation problem for illustration and testing"
scen = Scenario(
mp,
**models["dantzig"], # type: ignore [arg-type]
args = dict(
**models["dantzig"],
version="new",
annotation=annot,
scheme="dantzig",
with_data=True,
)
if request:
# Use a distinct scenario name for a particular test
args.update(scenario=request.node.name)
scen = Scenario(
mp,
**args, # type: ignore [arg-type]
)

# commit the scenario
scen.commit("Import Dantzig's transport problem for testing.")
Expand Down
10 changes: 0 additions & 10 deletions ixmp/testing/jupyter.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,6 @@ def run_notebook(nb_path, tmp_path, env=None, **kwargs):
import nbformat
from nbclient import NotebookClient

# Workaround for https://github.com/jupyter/nbclient/issues/85
if (
sys.version_info[0] == 3
and sys.version_info[1] >= 8
and sys.platform.startswith("win")
):
import asyncio

asyncio.set_event_loop_policy(asyncio.WindowsSelectorEventLoopPolicy())

# Read the notebook
nb = nbformat.read(nb_path, as_version=4)

Expand Down
8 changes: 4 additions & 4 deletions ixmp/tests/backend/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,14 +135,14 @@ def test_cache_invalidate(self, test_mp):

backend.cache_invalidate(ts, "par", "baz", dict(x=["x1", "x2"], y=["y1", "y2"]))

def test_del_ts(self, test_mp):
def test_del_ts(self, test_mp, request):
"""Test CachingBackend.del_ts()."""
# Since CachingBackend is an abstract class, test it via JDBCBackend
backend = test_mp._backend
backend: CachingBackend = test_mp._backend # type: ignore
cache_size_pre = len(backend._cache)

# Load data, thereby adding to the cache
s = make_dantzig(test_mp)
s = make_dantzig(test_mp, request=request)
s.par("d")

# Cache size has increased
Expand All @@ -155,7 +155,7 @@ def test_del_ts(self, test_mp):
s.__del__() # Force deletion of cached objects associated with `s`

# Delete the object; associated cache is freed
del s
backend.del_ts(s)

# Objects were invalidated/removed from cache
assert cache_size_pre == len(backend._cache)
94 changes: 51 additions & 43 deletions ixmp/tests/backend/test_jdbc.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
import logging
import os
import platform
import sys
from sys import getrefcount
from typing import Tuple

Expand Down Expand Up @@ -47,13 +46,7 @@ def test_jvm_warn(recwarn):
assert len(recwarn) == 0, recwarn.pop().message


@pytest.mark.flaky(
reruns=5,
rerun_delay=2,
condition="GITHUB_ACTIONS" in os.environ and platform.system() == "Windows",
reason="Flaky; see iiasa/ixmp#489",
)
def test_close(test_mp_f, capfd):
def test_close_default_logging(test_mp_f, capfd):
"""Platform.close_db() doesn't throw needless exceptions."""
# Use the session-scoped fixture to avoid affecting other tests in this file
mp = test_mp_f
Expand All @@ -68,9 +61,21 @@ def test_close(test_mp_f, capfd):
captured = capfd.readouterr()
assert captured.out == ""

# With log level INFO, a message is printed

def test_close_increased_logging(test_mp_f, capfd):
"""Platform.close_db() doesn't throw needless exceptions."""
# Use the session-scoped fixture to avoid affecting other tests in this file
mp = test_mp_f

# Close once
mp.close_db()

# Set higher log level INFO
level = mp.get_log_level()
mp.set_log_level(logging.INFO)

# Close again, once already closed
# With logging.INFO, a message is printed
mp.close_db()
captured = capfd.readouterr()
msg = "Database connection could not be closed or was already closed"
Expand Down Expand Up @@ -237,19 +242,16 @@ def test_invalid_properties_file(test_data_path):
ixmp.Platform(dbprops=test_data_path / "hsqldb.properties")


@pytest.mark.flaky(
reruns=5,
rerun_delay=2,
condition="GITHUB_ACTIONS" in os.environ and platform.system() == "Windows",
reason="Flaky; see iiasa/ixmp#489",
)
def test_connect_message(capfd, caplog):
msg = "connected to database 'jdbc:hsqldb:mem://ixmptest' (user: ixmp)..."
def test_connect_message(capfd, caplog, request):
msg = (
f"connected to database 'jdbc:hsqldb:mem://{request.node.name}_0' "
"(user: ixmp)..."
)

ixmp.Platform(
backend="jdbc",
driver="hsqldb",
url="jdbc:hsqldb:mem://ixmptest",
url=f"jdbc:hsqldb:mem://{request.node.name}_0",
log_level="INFO",
)

Expand All @@ -260,11 +262,14 @@ def test_connect_message(capfd, caplog):
# a previous run may have left the Java log level higher than INFO, in which
# case the Java Platform object would not write to stderr before set_log_level()
# in the above call. Try again now that the level is INFO:

msg = (
f"connected to database 'jdbc:hsqldb:mem://{request.node.name}_1' "
"(user: ixmp)..."
)
ixmp.Platform(
backend="jdbc",
driver="hsqldb",
url="jdbc:hsqldb:mem://ixmptest",
url=f"jdbc:hsqldb:mem://{request.node.name}_1",
)

# Instead, log messages are printed to stdout
Expand All @@ -273,15 +278,15 @@ def test_connect_message(capfd, caplog):


@pytest.mark.parametrize("arg", [True, False])
def test_cache_arg(arg):
def test_cache_arg(arg, request):
"""Test 'cache' argument, passed to CachingBackend."""
mp = ixmp.Platform(
backend="jdbc",
driver="hsqldb",
url="jdbc:hsqldb:mem://test_cache_false",
cache=arg,
)
scen = make_dantzig(mp)
scen = make_dantzig(mp, request=request)

# Maybe put something in the cache
scen.par("a")
Expand Down Expand Up @@ -339,8 +344,8 @@ def test_init(tmp_env, args, kwargs, action, kind, match):
ixmp.Platform(*args, **kwargs)


def test_gh_216(test_mp):
scen = make_dantzig(test_mp)
def test_gh_216(test_mp, request):
scen = make_dantzig(test_mp, request=request)

filters = dict(i=["seattle", "beijing"])

Expand Down Expand Up @@ -376,30 +381,32 @@ def test_verbose_exception(test_mp, exception_verbose_true):
assert "at.ac.iiasa.ixmp.Platform.getScenario" in exc_msg


@pytest.mark.xfail(
condition=sys.version_info.minor <= 10,
raises=AssertionError,
# See also test_base.TestCachingBackend.test_del_ts
reason="https://github.com/iiasa/ixmp/issues/463",
@pytest.mark.flaky(
reruns=5,
rerun_delay=2,
condition="GITHUB_ACTIONS" in os.environ and platform.system() == "Windows",
reason="Flaky; see iiasa/ixmp#543",
)
def test_del_ts():
def test_del_ts(request):
mp = ixmp.Platform(
backend="jdbc",
driver="hsqldb",
url="jdbc:hsqldb:mem:test_del_ts",
)

backend: ixmp.backend.jdbc.JDBCBackend = mp._backend # type: ignore

# Number of Java objects referenced by the JDBCBackend
N_obj = len(mp._backend.jindex)
N_obj = len(backend.jindex)

# Create a list of some Scenario objects
N = 8
scenarios = [make_dantzig(mp)]
scenarios = [make_dantzig(mp, request=request)]
for i in range(1, N):
scenarios.append(scenarios[0].clone(scenario=f"clone {i}"))
scenarios.append(scenarios[0].clone(scenario=f"{request.node.name} clone {i}"))

# Number of referenced objects has increased by 8
assert len(mp._backend.jindex) == N_obj + N
assert len(backend.jindex) == N_obj + N

# Pop and free the objects
for i in range(N):
Expand All @@ -414,22 +421,23 @@ def test_del_ts():
s_id = id(s)

# Underlying Java object
s_jobj = mp._backend.jindex[s]
s_jobj = backend.jindex[s]

# Now delete the Scenario object
del s
# del s # should work, but doesn't always resolve to s.__del__()
backend.del_ts(s)

# Number of referenced objects decreases by 1
assert len(mp._backend.jindex) == N_obj + N - (i + 1)
assert len(backend.jindex) == N_obj + N - (i + 1)
# ID is no longer in JDBCBackend.jindex
assert s_id not in mp._backend.jindex
assert s_id not in backend.jindex

# s_jobj is the only remaining reference to the Java object
assert getrefcount(s_jobj) - 1 == 1
del s_jobj

# Backend is again empty
assert len(mp._backend.jindex) == N_obj
assert len(backend.jindex) == N_obj


# NB coverage is omitted because this test is not included in the standard suite
Expand Down Expand Up @@ -648,8 +656,8 @@ def test_reload_cycle(
memory_usage("shutdown")


def test_docs(test_mp):
scen = make_dantzig(test_mp)
def test_docs(test_mp, request):
scen = make_dantzig(test_mp, request=request)
# test model docs
test_mp.set_doc("model", {scen.model: "Dantzig model"})
assert test_mp.get_doc("model") == {"canning problem": "Dantzig model"}
Expand All @@ -672,9 +680,9 @@ def test_docs(test_mp):
assert ex.value.args[0] == exp


def test_cache_clear(test_mp):
def test_cache_clear(test_mp, request):
"""Removing set elements causes the cache to be cleared entirely."""
scen = make_dantzig(test_mp)
scen = make_dantzig(test_mp, request=request)

# Load an item so that it is cached
d0 = scen.par("d")
Expand Down
4 changes: 2 additions & 2 deletions ixmp/tests/core/test_scenario.py
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ def test_filter_str(scen_empty):
assert_frame_equal(exp[["s", "value"]], obs[["s", "value"]])


def test_solve_callback(test_mp):
def test_solve_callback(test_mp, request):
"""Test the callback argument to Scenario.solve().

In real usage, callback() would compute some kind of convergence criterion. This
Expand All @@ -705,7 +705,7 @@ def test_solve_callback(test_mp):
equals an expected value, and the model has 'converged'.
"""
# Set up the Dantzig problem
scen = make_dantzig(test_mp)
scen = make_dantzig(test_mp, request=request)

# Solve the scenario as configured
solve_args = dict(model="dantzig", quiet=True)
Expand Down
2 changes: 1 addition & 1 deletion ixmp/tests/core/test_timeseries.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ def ts(self, request, mp):
node = hash(request.node.nodeid.replace("/", " "))
# Class of object to yield
cls = request.param
yield cls(mp, model=f"test-{node}", scenario="test", version="new")
yield cls(mp, model=f"test-{node}", scenario=f"test-{node}", version="new")

# Initialize TimeSeries
@pytest.mark.parametrize("cls", [TimeSeries, Scenario])
Expand Down
16 changes: 16 additions & 0 deletions ixmp/tests/data/test_check_single_model_access.properties
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Used by test_access.py

config.name = unit_test_db@local

jdbc.driver = org.hsqldb.jdbcDriver
jdbc.url = jdbc:hsqldb:mem:test_access
jdbc.user = ixmp
jdbc.pwd = ixmp

application.tag = IXSE_SR15
application.serverURL = http://localhost:8888

config.server.url = {auth_url}
config.server.config = DemoDB
config.server.username = service_user_dev
config.server.password = service_user_dev
Loading
Loading