While working on SKARA-1565 I've realized that the current test implementations of Issue and PullRequest (TestIssue and TestPullRequest) do not represent the behavior of their real implementation counterparts very well. For SKARA-1565, this discrepancy in behavior makes it impossible to write new tests for the poller functionality and at the same time keeping existing tests for any specific bot working when switching the bot to using the poller. Fixing the tests for this turned out to be a pretty large change, so I'm moving it to a separate bug to make reviews easier.
The main issue here is the difference in lifecycle between instances of the test classes and the real classes. In production, when querying an IssueProject for an Issue, what you get is basically an object instance which contains a snapshot of the external server side data for the issue. When you do another query, you get a new instance with potentially more up to date data. Some data is not included in the snapshot and will be dynamically fetched from the remote server on request, e.g. comments. Any method on the production implementation of Issue that adds or modifies data calls through to the external server and does not update the current snapshot.
In contrast to the above behavior, the TestIssue, when returned from the TestIssueProject, will be a new instance each time, but that instance shares most of the underlying data directly with all other instances of the "same" issue. So most additions or updates are immediately reflected in all instances of the "same" issue. For the poller tests, this makes it impossible to simulate having different snapshots returned at different times, and comparing the contents of them to figure out if something has changed. I'm also worried that we risk inadvertently building in assumptions in our bot code that relies on this behavior.
I want to fix this by changing TestIssue and TestPullRequest to better mimic the behavior of the production implementations. To achieve this, I'm introducing two (new) classes TestIssueStore and TestPullRequestStore. These classes represent the server side state of these objects. The TestIssue and TestPullRequest are changed to take similar snapshots as the production classes, and any method that modifies the state will just call through and modify the *Store instance. For test code to have an easier time inspecting "server side" data, the *Store objects are exposed to test code.
This will be a test only change that should not touch any product code. It will touch quite a lot of test code to adapt to the new behavior and API though.
The main issue here is the difference in lifecycle between instances of the test classes and the real classes. In production, when querying an IssueProject for an Issue, what you get is basically an object instance which contains a snapshot of the external server side data for the issue. When you do another query, you get a new instance with potentially more up to date data. Some data is not included in the snapshot and will be dynamically fetched from the remote server on request, e.g. comments. Any method on the production implementation of Issue that adds or modifies data calls through to the external server and does not update the current snapshot.
In contrast to the above behavior, the TestIssue, when returned from the TestIssueProject, will be a new instance each time, but that instance shares most of the underlying data directly with all other instances of the "same" issue. So most additions or updates are immediately reflected in all instances of the "same" issue. For the poller tests, this makes it impossible to simulate having different snapshots returned at different times, and comparing the contents of them to figure out if something has changed. I'm also worried that we risk inadvertently building in assumptions in our bot code that relies on this behavior.
I want to fix this by changing TestIssue and TestPullRequest to better mimic the behavior of the production implementations. To achieve this, I'm introducing two (new) classes TestIssueStore and TestPullRequestStore. These classes represent the server side state of these objects. The TestIssue and TestPullRequest are changed to take similar snapshots as the production classes, and any method that modifies the state will just call through and modify the *Store instance. For test code to have an easier time inspecting "server side" data, the *Store objects are exposed to test code.
This will be a test only change that should not touch any product code. It will touch quite a lot of test code to adapt to the new behavior and API though.
- blocks
-
SKARA-1565 Only poll for updated MRs from GitLab
- Resolved
- relates to
-
SKARA-1603 Make labels handling consistent in all Issue implementations
- Resolved