Open-MSS
fix: update abstract base class to include missing parameter
Purpose of PR?: Updates the abstract base class signature of _determine_filename to include a default reload parameter, aligning it with its implementations and usage in have_data.
Bug fix Fixes #2745
I was going for that initially, but didn’t import pytest at the time so I just quickly checked the behavior with a try-except. I have updated it, does this look good?
I think the error is caused when I clear _filetree (by setting it to {}), the expected ValueError isn’t raised maybe DUT’s setup or a caching mechanism might be preventing it from being empty. i am thinking of adding an assertion to verify the internal state before calling _determine_filename and use debug logging to see if the except clause is reached. That way if the implementation needs to be adjusted.
I think the error is caused when I clear _filetree (by setting it to {}), the expected ValueError isn’t raised maybe DUT’s setup or a caching mechanism might be preventing it from being empty.
The test failure you are seeing is not caused by the first with pytest.raises(ValueError):, but by the second one. Congratulations, you have found a new bug: _determine_filename returns an invalid/outdated filename with reload=True because it blindly trusts the cached filenames in _filetree. @ReimarBauer I think this needs some clarification on how this method is supposed to work, the reload parameter suggests there have been performance concerns in the past that prevented an implementation that tries to always be consistent with the filesystem?
In an attempted fix of this bug I noticed that just appending .bak to a filename in the test is not enough, as the next call to .setup will then pick up the variable from the .bak file.
Thanks. I hadn’t realized the failure was from the second ValueError. I’ll hold off on changes for now and wait for clarification on the expected behavior of reload. I’ll also update the test to avoid .bak files being picked up by .setup.