-
-
Notifications
You must be signed in to change notification settings - Fork 652
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
move the declarative task stuff out of the python backend testing #7279
move the declarative task stuff out of the python backend testing #7279
Conversation
single_product = all_products[0] | ||
return single_product | ||
|
||
def _create_task(self, task_type, context): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TaskTestBase
should handle this I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a docstring -- this is used specifically to hydrate the run_before_task_types
and run_after_task_types
in self.invoke_tasks()
.
Should have addressed the comments above, and finally managed to get a green CI run! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
raise NotImplementedError('dist_specs must be implemented!') | ||
|
||
@classproperty | ||
def run_before_task_types(cls): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The run_before
and run_after
tasks look very important for correct usage of this class, but it's not clear to me what they would do (without reading the implementation of this class).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added lots more documentation (and removed the previous docstrings)!
"""Tasks to run after local dists are built, similar to `run_before_task_types`.""" | ||
return [] | ||
|
||
def populate_target_dict(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This helper method doesn't seem like it is specific to this class. You could probably pass in the dist_specs
as a map, and return the created targets (which you could then pass to invoke_tasks
). That would allow the method to move up to TestBase
, and make it useful in more places.
The rest of the Task manipulation stuff could probably stay here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
tests/python/pants_test/test_base.py
Outdated
|
||
The values of `target_map` should themselves be a flat dict, which contains keyword arguments | ||
fed into `self.make_target()`, along with a few special keys. Special keys are: | ||
- 'key' (required -- used to index into the returned dict). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this be an address, or address string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It could default to that, actually -- the idea was to decouple accessing the target from within testing from its specific address so you could use a test class-specific shorthand.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It now defaults to the target address string!
187d7ff
to
c6a4031
Compare
Problem
We want to do #7128, which involves adding some more C/C++ unit testing. We would love to use some of the declarative task specification stuff we've added to the
python_dist()
testing to do that.Solution
BuildLocalPythonDistributionsTestBase
intoDeclarativeTaskTestMixin
intask_test_base.py
.Result
A new interface for task unit testing (which can be extended to more than just v1 tasks) is available to other testing.