Skip to content

Commit

Permalink
Applied general fixes from PR 667 (#720)
Browse files Browse the repository at this point in the history
# Description
Applied general improvements from PR 667.

The purpose of this PR is to ensure that the fixes in PR 667 are not
lost. This includes
- save test output in a separate directory (for some tests, see issue
#609)
- do not run test_property_mappings if rdflib is missing
- documented environment variable DLITE_ATEXIT_FREE
- fixed memory leak in test_bson_storage

Explanation of some of the changes can be found on #720

## Type of change
- [x] Bug fix & code cleanup
- [ ] New feature
- [ ] Documentation update
- [ ] Test update

## Checklist for the reviewer
This checklist should be used as a help for the reviewer.

- [ ] Is the change limited to one issue?
- [ ] Does this PR close the issue?
- [ ] Is the code easy to read and understand?
- [ ] Do all new feature have an accompanying new test?
- [ ] Has the documentation been updated as necessary?
  • Loading branch information
jesper-friis committed Dec 6, 2023
2 parents f1484cf + 0ba4df7 commit f23ddf8
Show file tree
Hide file tree
Showing 12 changed files with 71 additions and 16 deletions.
5 changes: 3 additions & 2 deletions bindings/python/tests/test_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

thisdir = Path(__file__).resolve().parent
rootdir = thisdir.parent.parent.parent
outdir = thisdir / "output"


class Person:
Expand All @@ -29,12 +30,12 @@ def __repr__(self):

print("-- create: person2")
person2 = ExPerson("Jack Daniel", 42, ["distilling", "tasting"])
person2.dlite_inst.save("json", "persons.json", "mode=w")
person2.dlite_inst.save("json", f"{outdir}/persons.json", "mode=w")

# Print json-representation of person2 using dlite
print(person2.dlite_inst.asjson(indent=2))

inst = dlite.Instance.from_url("json://persons.json")
inst = dlite.Instance.from_url(f"json://{outdir}/persons.json")
person3 = dlite.instancefactory(Person, inst)

person4 = dlite.objectfactory(person1, meta=person2.dlite_meta)
Expand Down
3 changes: 2 additions & 1 deletion bindings/python/tests/test_property_mappings.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
import numpy as np

try:
from tripper import Triplestore
import rdflib
from pint import Quantity
from tripper import Triplestore
except ImportError as exc:
sys.exit(44) # exit code marking the test to be skipped

Expand Down
5 changes: 4 additions & 1 deletion bindings/python/tests/test_pydantic.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ class TransformationStatus(BaseModel):
assert inst.startTime == int(now - 3000)
utc = timezone(timedelta(hours=0))
dt = datetime.fromtimestamp(now - 600).astimezone(utc)
assert inst.finishTime == str(dt)

# Timezone may is printed differently depending on Python version.
# Strip it off when testing finishTime
assert inst.finishTime.split("+")[0] == str(dt).split("+")[0]


# ==============================================================
Expand Down
15 changes: 8 additions & 7 deletions bindings/python/tests/test_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@


thisdir = os.path.abspath(os.path.dirname(__file__))
outdir = f"{thisdir}/output"

url = "json://" + thisdir + "/MyEntity.json"

Expand All @@ -35,26 +36,26 @@
inst["a-bool-array"] = True, False

# Test Storage.save()
with dlite.Storage("json", "tmp.json", "mode=w") as s:
with dlite.Storage("json", f"{outdir}/tmp.json", "mode=w") as s:
s.save(inst)

# Test query


# Test json
print("--- testing json")
myentity.save("json://myentity.json?mode=w")
inst.save("json://inst.json?mode=w")
myentity.save(f"json://{outdir}/myentity.json?mode=w")
inst.save(f"json://{outdir}/inst.json?mode=w")
del inst
inst = dlite.Instance.from_url(f"json://{thisdir}/inst.json#my-data")
inst = dlite.Instance.from_url(f"json://{outdir}/inst.json#my-data")


# Test yaml
if HAVE_YAML:
print('--- testing yaml')
inst.save('yaml://inst.yaml?mode=w')
inst.save(f"yaml://{outdir}/inst.yaml?mode=w")
del inst
inst = dlite.Instance.from_url("yaml://inst.yaml#my-data")
inst = dlite.Instance.from_url(f"yaml://{outdir}/inst.yaml#my-data")

# test help()
expected = """\
Expand All @@ -73,7 +74,7 @@
- `single`: Whether the input is assumed to be in single-entity form.
If "auto" (default) the form will be inferred automatically.
"""
s = dlite.Storage("yaml", "inst.yaml", options="mode=a")
s = dlite.Storage("yaml", f"{outdir}/inst.yaml", options="mode=a")
assert s.help().strip() == expected.strip()

# Test delete()
Expand Down
42 changes: 41 additions & 1 deletion cmake/valgrind-python.supp
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,47 @@
fun:_PyEval_EvalFrameDefault
}

{
Ignore conditional jump depends on uninitialised variable
Memcheck:Cond
fun:__wcsncpy_avx2
...
fun:_PyPathConfig_Calculate
...
fun:_PyPathConfig_Init
...
fun:_PyCoreConfig_Read
...
fun:_Py_UnixMain
}

{
Invalid read in main (py39)
Memcheck:Addr32
fun:__wcsncpy_avx2
fun:_PyPathConfig_Calculate
fun:_PyPathConfig_Init
fun:_PyCoreConfig_Read
...
fun:_Py_UnixMain
}

{
Invalid read in main (py311)
Memcheck:Addr32
fun:__wcsncpy_avx2
...
fun:_PyObject_MakeTpCall
fun:_PyEval_EvalFrameDefault
...
fun:PyEval_EvalCode
...
fun:Py_InitializeFromConfig
...
fun:Py_BytesMain
}


# ------------------------------------
# Python plugins
# ------------------------------------
Expand Down Expand Up @@ -967,7 +1008,6 @@
}



# ------------------------------------------
# valgrind suppressions generated from
# ts.log
Expand Down
1 change: 1 addition & 0 deletions doc/contributors_guide/tips_and_tricks.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Debugging tests failing inside docker on GitHub

3. To list all manylinux images for Python 3.7, do

cd dlite # Root of DLite source directory
CIBW_MANYLINUX_X86_64_IMAGE=ghcr.io/sintef/dlite-python-manylinux2014_x86_64:latest \
CIBW_BUILD=cp37-manylinux_* \
python -m cibuildwheel \
Expand Down
3 changes: 3 additions & 0 deletions doc/user_guide/environment_variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ DLite-specific environment variables
lost. But, if `DLITE_PYDEBUG` is defined, a Python error message will be
written to standard error.

- **DLITE_ATEXIT_FREE**: Free memory at exit. This might be useful to avoid
getting false positive when tracking down memory leaks with tools like valgrind.


### Specific paths
These environment variables can be used to provide additional search
Expand Down
4 changes: 3 additions & 1 deletion src/dlite-storage-plugins.c
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ const DLiteStoragePlugin *dlite_storage_plugin_get(const char *name)
r = asnpprintf(&buf, &size, m, " - %s\n", p);
if (r >= 0) m += r;
}
fu_endmatch(iter);

if ((failed_paths = dlite_python_storage_failed_paths())) {
r = asnpprintf(&buf, &size, m,
Expand All @@ -178,15 +179,16 @@ const DLiteStoragePlugin *dlite_storage_plugin_get(const char *name)
}
}

#endif
if (n <= 1)
m += asnpprintf(&buf, &size, m,
" Are the required Python packages installed or %s\n"
" DLITE_STORAGE_PLUGIN_DIRS or "
"DLITE_PYTHON_STORAGE_PLUGIN_DIRS\n"
" environment variables set?", submsg);
errx(1, "%s", buf);
#endif
free(buf);

}
return NULL;
}
Expand Down
4 changes: 2 additions & 2 deletions storages/json/dlite-json-storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -369,9 +369,9 @@ static DLiteStoragePlugin dlite_json_plugin = {


DSL_EXPORT const DLiteStoragePlugin *
get_dlite_storage_plugin_api(void *state, int *iter)
get_dlite_storage_plugin_api(void *globals, int *iter)
{
UNUSED(iter);
dlite_globals_set(state);
dlite_globals_set(globals);
return &dlite_json_plugin;
}
2 changes: 1 addition & 1 deletion storages/python/python-storage-plugins/mongodb.py
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def load(self, id):
document = self.collection.find_one({"uuid": uuid})
if not document:
raise IOError(
f"No instance with {uuid=} in MongoDB database "
f"No instance with uuid={uuid} in MongoDB database "
f'"{self.collection.database.name}" and collection '
f'"{self.collection.name}"'
)
Expand Down
2 changes: 2 additions & 0 deletions storages/python/tests-c/test_bson_storage.c
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ MU_TEST(test_load)
mu_assert_string_eq(json_str2, bson_str2);
free(json_str1);
free(json_str2);
free(bson_str1);
free(bson_str2);

stat = dlite_storage_close(s);
mu_assert_int_eq(0, stat);
Expand Down
1 change: 1 addition & 0 deletions storages/python/tests-python/test_mongodb_python.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import dlite

try:
import pymongo
import mongomock
except ImportError:
sys.exit(44) # skip test
Expand Down

0 comments on commit f23ddf8

Please sign in to comment.