Conversation
pengyunie
left a comment
pengyunie
left a comment
There was a problem hiding this comment.
Overall, the refactoring to the constructor parsing looks great. Please take out the diff inline test part from this PR though. Thanks.
src/inline/plugin.py
Outdated
| arg_disabled_str = "disabled" | ||
| arg_timeout_str = "timeout" | ||
| arg_devices_str = "devices" | ||
| diff_test_str = "diff_test" |
There was a problem hiding this comment.
Can you take these (and other places related to devices and diff_test) out of this PR? They belong to the diff inline test functionality, so should be moved to a later PR instead.
| raise MalformedException( | ||
| f"inline test: {self.class_name_str}() accepts {NUM_OF_ARGUMENTS} arguments. 'test_name' must be a string constant, 'parameterized' must be a boolean constant, 'repeated' must be a positive integer, 'tag' must be a list of string, 'timeout' must be a positive float" | ||
| ) | ||
|
|
There was a problem hiding this comment.
Cool, it looks so much cleaner now.
We may keep the lowest supported Python version to be lower than 3.10 for a while, so cannot completely switch to match for now...
src/inline/plugin.py
Outdated
| class InlineTestRunner: | ||
| def run(self, test: InlineTest, out: List) -> None: | ||
| test_str = test.to_test() | ||
| print(test_str) |
There was a problem hiding this comment.
If this print is used for only debugging, please remove
tests/test_plugin.py
Outdated
| # For testing in Spyder only | ||
| # if __name__ == "__main__": | ||
| # pytest.main(['-v', '-s']) | ||
|
|
There was a problem hiding this comment.
I would suggest not committing this kind of IDE-specific debugging code snippets into the codebase. Better to keep it in a separate notetaking app.
src/inline/plugin.py
Outdated
| self.cur_inline_test.check_stmts = comparisons | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
(Same comment as before) please split it into a future PR for diff inline test.
- Removed IDE Debug statements - Removed Diff_test functionality; will be added back later in future PR
hanse7962
left a comment
hanse7962
left a comment
There was a problem hiding this comment.
Changes implemented based on feedback:
- Removed diff_test functionality. Will be added back in future PR.
- Removed IDE Debug code
2 participants
(Note: this is a duplicate of a previous pull request. Repo was unliked and reforked to remove undesired changes from original fork.)
Inline Test constructor has been refactored to improve efficiency: