From d6d6bf1ac743a811e5722e4f9cbe62de4aa80d6d Mon Sep 17 00:00:00 2001 From: Chris Kerr Date: Tue, 2 Apr 2019 21:34:34 +0200 Subject: [PATCH 1/7] Use a pytest fixture to locate the testdata directory --- tests/conftest.py | 11 +++++++++++ tests/test_BioLogic.py | 20 +++++++++----------- 2 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 tests/conftest.py diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..1d2e5ad --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,11 @@ +"""Helpers for pytest tests.""" + +import os + +import pytest + + +@pytest.fixture(scope='session') +def testdata_dir(): + """Path to the testdata directory.""" + return os.path.join(os.path.dirname(__file__), 'testdata') diff --git a/tests/test_BioLogic.py b/tests/test_BioLogic.py index f42e85d..fc504e6 100644 --- a/tests/test_BioLogic.py +++ b/tests/test_BioLogic.py @@ -11,10 +11,8 @@ import pytest from galvani import MPTfile, MPRfile from galvani.BioLogic import MPTfileCSV, str3 # not exported -testdata_dir = os.path.join(os.path.dirname(__file__), 'testdata') - -def test_open_MPT(): +def test_open_MPT(testdata_dir): mpt1, comments = MPTfile(os.path.join(testdata_dir, 'bio_logic1.mpt')) assert comments == [] assert mpt1.dtype.names == ( @@ -24,12 +22,12 @@ def test_open_MPT(): ) -def test_open_MPT_fails_for_bad_file(): +def test_open_MPT_fails_for_bad_file(testdata_dir): with pytest.raises(ValueError, match='Bad first line'): MPTfile(os.path.join(testdata_dir, 'bio_logic1.mpr')) -def test_open_MPT_csv(): +def test_open_MPT_csv(testdata_dir): mpt1, comments = MPTfileCSV(os.path.join(testdata_dir, 'bio_logic1.mpt')) assert comments == [] assert mpt1.fieldnames == [ @@ -39,7 +37,7 @@ def test_open_MPT_csv(): ] -def test_open_MPT_csv_fails_for_bad_file(): +def test_open_MPT_csv_fails_for_bad_file(testdata_dir): with pytest.raises((ValueError, UnicodeDecodeError)): MPTfileCSV(os.path.join(testdata_dir, 'bio_logic1.mpr')) @@ -55,7 +53,7 @@ def test_open_MPT_csv_fails_for_bad_file(): # C019P-0ppb-A_C01.mpr stores the date in a different format ('C019P-0ppb-A_C01.mpr', '2019-03-14', '2019-03-14'), ]) -def test_MPR_dates(filename, startdate, enddate): +def test_MPR_dates(testdata_dir, filename, startdate, enddate): """Check that the start and end dates in .mpr files are read correctly.""" mpr = MPRfile(os.path.join(testdata_dir, filename)) assert mpr.startdate.strftime('%Y-%m-%d') == startdate @@ -65,7 +63,7 @@ def test_MPR_dates(filename, startdate, enddate): assert not hasattr(mpr, 'enddate') -def test_open_MPR_fails_for_bad_file(): +def test_open_MPR_fails_for_bad_file(testdata_dir): with pytest.raises(ValueError, match='Invalid magic for .mpr file'): MPRfile(os.path.join(testdata_dir, 'arbin1.res')) @@ -134,7 +132,7 @@ def assert_MPR_matches_MPT(mpr, mpt, comments): 'CV_C01', '121_CA_455nm_6V_30min_C01', ]) -def test_MPR_matches_MPT(basename): +def test_MPR_matches_MPT(testdata_dir, basename): """Check the MPR parser against the MPT parser. Load a binary .mpr file and a text .mpt file which should contain @@ -147,7 +145,7 @@ def test_MPR_matches_MPT(basename): assert_MPR_matches_MPT(mpr, mpt, comments) -def test_MPR5_matches_MPT5(): +def test_MPR5_matches_MPT5(testdata_dir): mpr = MPRfile(os.path.join(testdata_dir, 'bio_logic5.mpr')) mpt, comments = MPTfile((re.sub(b'\tXXX\t', b'\t0\t', line) for line in open(os.path.join(testdata_dir, 'bio_logic5.mpt'), @@ -155,7 +153,7 @@ def test_MPR5_matches_MPT5(): assert_MPR_matches_MPT(mpr, mpt, comments) -def test_MPR6_matches_MPT6(): +def test_MPR6_matches_MPT6(testdata_dir): mpr = MPRfile(os.path.join(testdata_dir, 'bio_logic6.mpr')) mpt, comments = MPTfile(os.path.join(testdata_dir, 'bio_logic6.mpt')) mpr.data = mpr.data[:958] # .mpt file is incomplete From 5530a7a8ff1360a0517cf1cc994283fe6f17732e Mon Sep 17 00:00:00 2001 From: Chris Kerr Date: Tue, 2 Apr 2019 22:05:41 +0200 Subject: [PATCH 2/7] Add a simple test for loading Arbin .res files --- tests/test_Arbin.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 tests/test_Arbin.py diff --git a/tests/test_Arbin.py b/tests/test_Arbin.py new file mode 100644 index 0000000..9ceec8e --- /dev/null +++ b/tests/test_Arbin.py @@ -0,0 +1,26 @@ +"""Tests for loading Arbin .res files.""" + +import os +import sqlite3 +import subprocess + +import pytest + +from galvani import res2sqlite + + +# TODO - change to subprocess.DEVNULL when python 2 support is removed +have_mdbtools = (subprocess.call(['which', 'mdb-export'], stdout=None) == 0) + + +@pytest.mark.skipif(not have_mdbtools, reason='Reading the Arbin file requires MDBTools') +@pytest.mark.parametrize('basename', ['arbin1']) +def test_convert_Arbin_to_sqlite(testdata_dir, tmpdir, basename): + """Convert an Arbin file to SQLite using the functional interface.""" + res_file = os.path.join(testdata_dir, basename + '.res') + sqlite_file = os.path.join(str(tmpdir), basename + '.s3db') + res2sqlite.convert_arbin_to_sqlite(res_file, sqlite_file) + assert os.path.isfile(sqlite_file) + with sqlite3.connect(sqlite_file) as conn: + csr = conn.execute('SELECT * FROM Channel_Normal_Table;') + csr.fetchone() From a1b73867ea9a3cc50572ad779c12630bb6651176 Mon Sep 17 00:00:00 2001 From: Chris Kerr Date: Tue, 2 Apr 2019 22:06:26 +0200 Subject: [PATCH 3/7] Add a test that a sensible error is raised when MDBTools is not found This is the error that happens in issue #23 --- tests/test_Arbin.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/test_Arbin.py b/tests/test_Arbin.py index 9ceec8e..97c34a7 100644 --- a/tests/test_Arbin.py +++ b/tests/test_Arbin.py @@ -3,6 +3,7 @@ import os import sqlite3 import subprocess +import sys import pytest @@ -13,6 +14,19 @@ from galvani import res2sqlite have_mdbtools = (subprocess.call(['which', 'mdb-export'], stdout=None) == 0) +@pytest.mark.skipif(have_mdbtools, reason='This tests the failure when mdbtools is not installed') +def test_convert_Arbin_no_mdbtools(testdata_dir, tmpdir): + """Checks that the conversion fails with an appropriate error message.""" + res_file = os.path.join(testdata_dir, 'arbin1.res') + sqlite_file = os.path.join(str(tmpdir), 'arbin1.s3db') + if sys.version_info >= (3, 3): + expected_exception = FileNotFoundError + else: + expected_exception = OSError + with pytest.raises(expected_exception, match="No such file or directory: 'mdb-export'"): + res2sqlite.convert_arbin_to_sqlite(res_file, sqlite_file) + + @pytest.mark.skipif(not have_mdbtools, reason='Reading the Arbin file requires MDBTools') @pytest.mark.parametrize('basename', ['arbin1']) def test_convert_Arbin_to_sqlite(testdata_dir, tmpdir, basename): From 557e755f03e4706aca1ba3cadd043e7677d4427e Mon Sep 17 00:00:00 2001 From: Chris Kerr Date: Tue, 2 Apr 2019 23:09:53 +0200 Subject: [PATCH 4/7] Move Popen call outside the try/finally block Ensure that all variables used in the except and finally blocks are always defined - fixes #23 In Python 3, Popen objects can be used as contextmanagers, but not in Python 2.7 --- galvani/res2sqlite.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/galvani/res2sqlite.py b/galvani/res2sqlite.py index 0234a7b..92cf630 100755 --- a/galvani/res2sqlite.py +++ b/galvani/res2sqlite.py @@ -352,10 +352,14 @@ CREATE VIEW IF NOT EXISTS Capacity_View def mdb_get_data_text(s3db, filename, table): print("Reading %s..." % table) + # TODO after dropping Python 2 support - use Popen as contextmanager + mdb_sql = sp.Popen(['mdb-export', '-I', 'postgres', filename, table], + bufsize=-1, stdin=None, stdout=sp.PIPE, + universal_newlines=True) try: - mdb_sql = sp.Popen(['mdb-export', '-I', 'postgres', filename, table], - bufsize=-1, stdin=None, stdout=sp.PIPE, - universal_newlines=True) + # Initialize values to avoid NameError in except clause + mdb_output = '' + insert_match = None mdb_output = mdb_sql.stdout.read() while len(mdb_output) > 0: insert_match = re.match(r'INSERT INTO "\w+" \([^)]+?\) VALUES \(("[^"]*"|[^")])+?\);\n', @@ -365,7 +369,8 @@ def mdb_get_data_text(s3db, filename, table): s3db.commit() except: print("Error while importing %s" % table) - print("Remaining mdb-export output:", mdb_output) + if mdb_output: + print("Remaining mdb-export output:", mdb_output) if insert_match: print("insert_re match:", insert_match) raise @@ -375,10 +380,11 @@ def mdb_get_data_text(s3db, filename, table): def mdb_get_data_numeric(s3db, filename, table): print("Reading %s..." % table) + # TODO after dropping Python 2 support - use Popen as contextmanager + mdb_sql = sp.Popen(['mdb-export', filename, table], + bufsize=-1, stdin=None, stdout=sp.PIPE, + universal_newlines=True) try: - mdb_sql = sp.Popen(['mdb-export', filename, table], - bufsize=-1, stdin=None, stdout=sp.PIPE, - universal_newlines=True) mdb_csv = csv.reader(mdb_sql.stdout) mdb_headers = next(mdb_csv) quoted_headers = ['"%s"' % h for h in mdb_headers] From 6a8fbe71a4575b48f4528a5d4b0244f1c87e0831 Mon Sep 17 00:00:00 2001 From: Chris Kerr Date: Tue, 2 Apr 2019 23:01:32 +0200 Subject: [PATCH 5/7] Add some tests for the `res2sqlite` command-line tool Check that the --help option works even if mdbtools is not installed --- tests/test_Arbin.py | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tests/test_Arbin.py b/tests/test_Arbin.py index 97c34a7..9032a9b 100644 --- a/tests/test_Arbin.py +++ b/tests/test_Arbin.py @@ -14,6 +14,15 @@ from galvani import res2sqlite have_mdbtools = (subprocess.call(['which', 'mdb-export'], stdout=None) == 0) +def test_res2sqlite_help(): + """Test running `res2sqlite --help`. + + This should work even when mdbtools is not installed. + """ + help_output = subprocess.check_output(['res2sqlite', '--help']) + assert b'Convert Arbin .res files to sqlite3 databases' in help_output + + @pytest.mark.skipif(have_mdbtools, reason='This tests the failure when mdbtools is not installed') def test_convert_Arbin_no_mdbtools(testdata_dir, tmpdir): """Checks that the conversion fails with an appropriate error message.""" @@ -29,7 +38,7 @@ def test_convert_Arbin_no_mdbtools(testdata_dir, tmpdir): @pytest.mark.skipif(not have_mdbtools, reason='Reading the Arbin file requires MDBTools') @pytest.mark.parametrize('basename', ['arbin1']) -def test_convert_Arbin_to_sqlite(testdata_dir, tmpdir, basename): +def test_convert_Arbin_to_sqlite_function(testdata_dir, tmpdir, basename): """Convert an Arbin file to SQLite using the functional interface.""" res_file = os.path.join(testdata_dir, basename + '.res') sqlite_file = os.path.join(str(tmpdir), basename + '.s3db') @@ -38,3 +47,15 @@ def test_convert_Arbin_to_sqlite(testdata_dir, tmpdir, basename): with sqlite3.connect(sqlite_file) as conn: csr = conn.execute('SELECT * FROM Channel_Normal_Table;') csr.fetchone() + + +@pytest.mark.skipif(not have_mdbtools, reason='Reading the Arbin file requires MDBTools') +def test_convert_cmdline(testdata_dir, tmpdir): + """Checks that the conversion fails with an appropriate error message.""" + res_file = os.path.join(testdata_dir, 'arbin1.res') + sqlite_file = os.path.join(str(tmpdir), 'arbin1.s3db') + subprocess.check_call(['res2sqlite', res_file, sqlite_file]) + assert os.path.isfile(sqlite_file) + with sqlite3.connect(sqlite_file) as conn: + csr = conn.execute('SELECT * FROM Channel_Normal_Table;') + csr.fetchone() From 846a5b31491f9bfbf1e4ba155345c4d994aaa3c4 Mon Sep 17 00:00:00 2001 From: Chris Kerr Date: Wed, 3 Apr 2019 08:13:01 +0200 Subject: [PATCH 6/7] Catch FileNotFoundError from Popen and re-raise a more helpful message --- galvani/res2sqlite.py | 26 ++++++++++++++++++++------ tests/test_Arbin.py | 7 +------ 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/galvani/res2sqlite.py b/galvani/res2sqlite.py index 92cf630..c2fab8d 100755 --- a/galvani/res2sqlite.py +++ b/galvani/res2sqlite.py @@ -353,9 +353,16 @@ CREATE VIEW IF NOT EXISTS Capacity_View def mdb_get_data_text(s3db, filename, table): print("Reading %s..." % table) # TODO after dropping Python 2 support - use Popen as contextmanager - mdb_sql = sp.Popen(['mdb-export', '-I', 'postgres', filename, table], - bufsize=-1, stdin=None, stdout=sp.PIPE, - universal_newlines=True) + try: + mdb_sql = sp.Popen(['mdb-export', '-I', 'postgres', filename, table], + bufsize=-1, stdin=None, stdout=sp.PIPE, + universal_newlines=True) + except OSError as e: + if e.errno == 2: + raise RuntimeError('Could not locate the `mdb-export` executable. ' + 'Check that mdbtools is properly installed.') + else: + raise try: # Initialize values to avoid NameError in except clause mdb_output = '' @@ -381,9 +388,16 @@ def mdb_get_data_text(s3db, filename, table): def mdb_get_data_numeric(s3db, filename, table): print("Reading %s..." % table) # TODO after dropping Python 2 support - use Popen as contextmanager - mdb_sql = sp.Popen(['mdb-export', filename, table], - bufsize=-1, stdin=None, stdout=sp.PIPE, - universal_newlines=True) + try: + mdb_sql = sp.Popen(['mdb-export', filename, table], + bufsize=-1, stdin=None, stdout=sp.PIPE, + universal_newlines=True) + except OSError as e: + if e.errno == 2: + raise RuntimeError('Could not locate the `mdb-export` executable. ' + 'Check that mdbtools is properly installed.') + else: + raise try: mdb_csv = csv.reader(mdb_sql.stdout) mdb_headers = next(mdb_csv) diff --git a/tests/test_Arbin.py b/tests/test_Arbin.py index 9032a9b..f21bd50 100644 --- a/tests/test_Arbin.py +++ b/tests/test_Arbin.py @@ -3,7 +3,6 @@ import os import sqlite3 import subprocess -import sys import pytest @@ -28,11 +27,7 @@ def test_convert_Arbin_no_mdbtools(testdata_dir, tmpdir): """Checks that the conversion fails with an appropriate error message.""" res_file = os.path.join(testdata_dir, 'arbin1.res') sqlite_file = os.path.join(str(tmpdir), 'arbin1.s3db') - if sys.version_info >= (3, 3): - expected_exception = FileNotFoundError - else: - expected_exception = OSError - with pytest.raises(expected_exception, match="No such file or directory: 'mdb-export'"): + with pytest.raises(RuntimeError, match="Could not locate the `mdb-export` executable."): res2sqlite.convert_arbin_to_sqlite(res_file, sqlite_file) From 4381b02242673187dfd998a34d291c709d3490f6 Mon Sep 17 00:00:00 2001 From: Chris Kerr Date: Wed, 3 Apr 2019 08:16:55 +0200 Subject: [PATCH 7/7] Add .pytest_cache to Travis cache --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index e84d8f2..c4301f2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,6 +3,7 @@ language: python cache: directories: - .tox + - .pytest_cache - tests/testdata python: - "2.7"