Update GScan incrementally, using tree/lookup #802
Conversation
|
Yesterday: 18 failed tests And this is only unit tests. Still pending the e2e tests 😵 |
|
Unit tests fixed locally. Now figure out how to fix these conflicts 😰 |
… common module (so GScan can re-use it too)
…ted, use more props
|
Rebased, fixed conflicts, but couldn't figure out how to update the tree/gscan Vuex modules 💫 😵 |
Codecov Report
@@ Coverage Diff @@
## master #802 +/- ##
==========================================
- Coverage 91.00% 90.29% -0.71%
==========================================
Files 87 90 +3
Lines 1700 1773 +73
Branches 105 106 +1
==========================================
+ Hits 1547 1601 +54
- Misses 123 141 +18
- Partials 30 31 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Finally unit tests passing! 🙏 (also had to fix git conflicts, lint, then docs, so not only unit tests 😅 ) |
|
Just one e2e test failing now, almost there... |
| @@ -15,32 +15,81 @@ | |||
| * along with this program. If not, see <http://www.gnu.org/licenses/>. | |||
| */ | |||
|
|
|||
| import { sortedIndexBy } from '@/components/cylc/common/sort' | |||
| import { sortWorkflowNamePartNodeOrWorkflowNode } from '@/components/cylc/gscan/sort' | |||
| // TODO: move to the `options` parameter that is passed to deltas; ideally it would be stored in DB or localstorage. | |||
There was a problem hiding this comment.
This code is similar to what we have in the tree nodes.js
| * You should have received a copy of the GNU General Public License | ||
| * along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
| */ | ||
| import Vue from 'vue' |
There was a problem hiding this comment.
Equivalent of the tree component's CylcTree, but with hierarchy for GScan.
|
One brittle test. When we visit URL's, and then click on buttons, it's taking too long to load the subscription (sometimes) and then the test is failing as it did for firefox ☝️ will investigate tomorrow. |
|
The test was waiting for But there was an easier way to tell Cypress to wait for the tree to show. Just wait on This might take a while to review, so @AaronDCole will probably inherit it. But at least it's not brittle, and doesn't have git conflicts. The best way to understand this PR is to understand how the Tree component & view work. This is basically the same as GScan on 🖖 |
|
@oliver-sanders I'm guessing this will be closed now? |
|
I think we'll have to work out how to pull the propagate states logic out onto the redesigned store. I think we could probably put that logic into a computed property on the GScan component? |
|
Since the data store refactor we now have a universal tree containing both the GScan (workflow) and Tree (task) parts. This is updated incrementally using the global-callback. So the workflow tree is being computed incrementally, but GScan is still using a computed property rather than iterating over this directly because of sorting and filtering. In order to sort/filter incrementally it is still necessary to write a callback for GScan using the data from the lookup as done on this branch. The callbacks here are still valid, Because GScan is the only component (so far) that is worried about workflow iteration order, we could consider changing the sort order of the central store to what GScan expects, then use Alternatively, we could consider maintaining a separate index in the central store just for GScan (e.g. |
This is a small change with no associated Issue.
The PR to propagate states in GScan has two changes. One where we propagate states, and another where we replace the
computedproperty by a Tree+Lookup approach to avoid re-creating thecomputedproperty any time something changes in GScan, instead building the tree incrementally.To simplify the other PR, I have moved the latter over to this PR, so we can compare before/after and merge it first, especially since I didn't get good performance with the propagate states part (which means it could take a bit longer for that change to be ready for review.)
Requirements check-list
CONTRIBUTING.mdand added my name as a Code Contributor.