Fix #144 #147
Fix #144 #147
Conversation
| try: | ||
| self.dep_manager.save_success(task, result_hash=result) | ||
| except OSError as e: | ||
| write("failed {} ({})\n".format(task.name, e)) |
There was a problem hiding this comment.
This the Error message will be different than the your use-case when you removed only line_3.txt.
I would prefer if we explicitly check all file_deps exist. So we wont even need to check for res.status == "error".
|
Sorry about the commit spam - tests are passing on my local PC but not on Appveyor so I'm having trouble debugging. I'm still not sure what the problem is, one test is complaining that the database is in use by another process...? |
|
no worries about spam 😁 Looks like the error is not caused by you. just ignore it. |
|
i just re-run an old commit (that was successful), it now fails. So the problem must be some change on appveyor... Of course you are welcome to fix the issue. But better do this in another PR. |
|
OK great! Let me know if you're OK with this PR and I'll squash the commits. On 16 Sep 2016 5:32 p.m., "eduardo naufel schettino" <
|
| @@ -65,7 +71,9 @@ def _execute(self, pos_args=None): | |||
| continue | |||
|
|
|||
| task.values = values | |||
|
|
|||
There was a problem hiding this comment.
i like a lot of blank lines but this seems excessive.
| @@ -31,6 +31,22 @@ def remove_dependency(): | |||
|
|
|||
| # fixture to create a sample file to be used as file_dep | |||
| @pytest.fixture | |||
| def dependency2(request): | |||
There was a problem hiding this comment.
Copy and paste here is not acceptable.
Note that removing copy and paste here will NOT increase the complexity in the test itself (this code is on conftest).
Hopefully it will not sound contradictory or arbitrary comparing to my previous review.
| # Skip exception when a depencency file is missing, and force | ||
| # the state computation | ||
| write("failed {} ({})\n".format(task.name, res.get_error_message())) | ||
| missing_deps = [] |
There was a problem hiding this comment.
i guess this is a good candidate for a single line list-comprehension.
|
I'm guessing that this sort of factory approach is the kind of thing you were thinking about for generating multiple similar fixtures? I couldn't find much documentation about how to do this with pytest so not sure if I'm missing a more obvious solution here... |
|
merged thanks. sorry for delay. i forgot this one. |
Fix for issue #144
Catch the exception thrown when dependencies and targets are both missing. Also added regression test for this issue.