-
Notifications
You must be signed in to change notification settings - Fork 20
Added scoped test data implementation. #126
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
base: main
Are you sure you want to change the base?
Conversation
See example for usage
|
Black formatting errors raised in psf/black#1708 |
|
Hello Stranger! I've gone back and forth with 3 general concepts:
Below is part of an idea I was kicking around. The idea is to inject a reference to the sequencer to TestList and TestClass. Instead of using a direct reference like I have there, I would actually create some kind of proxy that only exposes the api which the sequencer want to provide. My gut feel is that this is a good general approach that keep the coupling nicely defined, but with potential to expand when needed for other purposes. (e.g. adding a get_script_params interface and getting rid of context_data entirely). However this doesn't allow for the pytest magic style function params. Any thoughts? For this shared data idea, the test script wouldn't directly access the _seq attribute, but there would be methods or properties on the test class (provided by the base class) which would use _seq (via a get_stack method?) to look for the shared data. |
|
Perhaps we should move this discussion (or port the discussions to date) to an issue. For your final point, The sequencer is already globally accessible through the RESOURCES["SEQUENCER"]. get_script_params method could be implemented without touching the sequencer logic, unless we remove the global scoping of the sequencer as mentioned above. I agree that context_data needs to go, it has too many disadvantages. As for limiting the exact parameter you are looking for in this PRs implementation. There are a few ways to go about this def test(self, outer: TestListOuter.outer, outer_other_data: TestListOuter.other_data):This doesn't work because the whole point is to share data between tests and this would only work if the value is already set and is mutable like a dict. 2 def test(self, outer: "TestListOuter.outer", outer_other_data: "TestListOuter.other_data"):This can share data but the type hinting is stuffed 3 def test(self, outer: TestListOuter, other_data: TestListOuter):Could use the name of the variable to indicate which parameter in the test list instance to parse. 4 @test_data(outer="TestListOuter.outer", other_data="TestListOuter.other_data")
class TestBoth(TestClass):
def test(self):
print(self.test_data.outer)
print(self.test_data.other_data)Explicitly defines the data parsed but you lose type hinting and auto complete. The problem is that you have to reference by string the TestLists in the @test_data because the data might be immutable or not exist yet. Eg if TestListOuter.outer is an int, then by the time it runs it could be set to another number 5 @test_data(outer="TestListOuter.outer", other_data="TestListOuter.other_data")
class TestBoth(TestClass):
def test(self, outer: int, other_data: dict):
passThis is probably the best alternative |
I can't reply to everything right now, but yes, you hit on my motivation to suggest doing it that way. The RESOURCES dict needs to go eventually and this would be a baby-step in that direction. |
@pazzarpj I just pinned the version of black on CI to work around this. If you continue on this PR you can grab that from master. |
Features:
Implementation Details: