usnistgov / mass

Microcalorimeter pulse-analysis software
MIT License
6 stars 0 forks source link

improve and expand usage of tempfile API to fix unreliable tests #284

Closed ggggggggg closed 4 months ago

ggggggggg commented 4 months ago

tempfile.TemporaryDirectory() does not create a directory, it creates an object that will create a directory when "enetered", so it needs to be used with a with statement or with an explicit enter. On the other hand with tempfile.NamedTemporaryFile as destfile opens destfile, so we if want to create a file with destfile.name, we need to close destfile first. On my windows pc all of these threw up errors 100% of the time, so there is some platform variation in how forgiving all this temp file stuff is. I believe I've switch to more correct uses of the API. Also in some cases the same temprorary dir was used twice in a row, so I used two different dirs.

test_ljh_copy_and_append_traces fails on windows with the error

C:\Users\oneilg\Desktop\python\src\mass>pytest --pdb tests/core/test_core.py
================================================ test session starts =================================================
platform win32 -- Python 3.10.11, pytest-7.4.1, pluggy-1.3.0
rootdir: C:\Users\oneilg\Desktop\python\src\mass
plugins: anyio-4.0.0, asyncio-0.21.1
asyncio: mode=strict
collected 27 items

tests\core\test_core.py F
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> traceback >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

    @staticmethod
    def test_ljh_copy_and_append_traces():
        """Test copying and appending traces to LJH files."""
        src_name = os.path.join('tests', 'regression_test', 'regress_chan1.ljh')
        src = LJHFile.open(src_name)
        with tempfile.NamedTemporaryFile(suffix="_chan1.ljh") as destfile:
            dest_name = destfile.name
            destfile.close()
            source_traces = [20]
>           ljh_copy_traces(src_name, dest_name, source_traces, overwrite=True)

tests\core\test_core.py:32:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
mass\core\ljh_modify.py:141: in ljh_copy_traces
    helper_write_pulse(dest_fp, src, i)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

dest = <_io.BufferedWriter name='C:\\Users\\oneilg\\AppData\\Local\\Temp\\tmplzsrk8hc_chan1.ljh'>
src = LJHFile.open('tests\regression_test\regress_chan1.ljh'), i = 20

    def helper_write_pulse(dest, src, i):
        subframecount, timestamp_usec, trace = src.read_trace_with_timing(i)
        prefix = struct.pack('<Q', int(subframecount))
        dest.write(prefix)
>       prefix = struct.pack('<Q', int(timestamp_usec))
E       struct.error: argument out of range

mass\core\ljh_modify.py:108: error
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> entering PDB >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> PDB post_mortem (IO-capturing turned off) >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> c:\users\oneilg\desktop\python\src\mass\mass\core\ljh_modify.py(108)helper_write_pulse()
-> prefix = struct.pack('<Q', int(timestamp_usec))
(Pdb) subframecount
32753611522
(Pdb) timestamp_usec
-2147483648

Where you can see that I've used the debugger to print out timestamp_usec which is negative. <Q is for unsigned ints, so it throws an error. Since this has long passed linux and our automated testing, I'm not going to to try just submitting this as a PR and see if it passes on github actions.

joefowler commented 4 months ago

Fixes #272. (We hope.)