Toolbar play from stopped

[oliver-sanders]

built on #805

This completes the play-pause/resume-stop sequence in the workflow toolbar.

Requirements check-list

• I have read CONTRIBUTING.md and added my name as a Code Contributor.
• Contains logically grouped changes (else tidy your branch by rebase).
• Does not contain off-topic changes (use other PRs for other changes).
• Appropriate tests are included (unit and/or functional) - hard to test with mocked data
• Appropriate change log entry included.
• No documentation update required.

[codecov-commenter]

Codecov Report

Merging #806 (d9960a4) into master (a05f013) will decrease coverage by 0.26%.
The diff coverage is 28.57%.

@@            Coverage Diff             @@
##           master     #806      +/-   ##
==========================================
- Coverage   91.69%   91.43%   -0.27%     
==========================================
  Files          86       86              
  Lines        1686     1693       +7     
  Branches      105      108       +3     
==========================================
+ Hits         1546     1548       +2     
- Misses        117      120       +3     
- Partials       23       25       +2     

Flag	Coverage Δ
e2e	79.62% <28.57%> (-0.22%)	⬇️
unittests	78.37% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/components/cylc/workflow/Toolbar.vue	73.07% <28.57%> (-16.40%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a05f013...d9960a4. Read the comment docs.

[kinow]

Looks good to me, pending only a test for the new methods added in the components 😬 unit tests can be used to access those properties, see:

• The mount function that creates components with access to Vuetify, Vuex, etc, and where you can also provide stubs/mocks:

  cylc-ui/tests/unit/components/cylc/gscan/gscan.vue.spec.js

  Lines 72 to 83 in a3005b0

  	const mountFunction = options => {
  	const vuetify = new Vuetify()
  	return mount(GScan, {
  	localVue,
  	vuetify,
  	store,
  	stubs: {
  	RouterLink: RouterLinkStub
  	},
  	...options
  	})
  	}

• The tests before/after callbacks in case you need to reset state before/after each test:

  cylc-ui/tests/unit/components/cylc/gscan/gscan.vue.spec.js

  Lines 62 to 67 in a3005b0

  	const resetState = () => {
  	store.commit('workflows/SET_WORKFLOWS', [])
  	store.commit('workflows/SET_WORKFLOW_NAME', null)
  	}
  	beforeEach(resetState)
  	afterEach(resetState)

• Calling the mount function, passing component options, which can include computed, propsOptions, etc:

  cylc-ui/tests/unit/components/cylc/gscan/gscan.vue.spec.js

  Lines 85 to 91 in a3005b0

  	const wrapper = mountFunction({
  	computed: {
  	isLoading () {
  	return true
  	}
  	}
  	})

• Accessing DOM attributes of the mounted component:

  cylc-ui/tests/unit/components/cylc/gscan/gscan.vue.spec.js

  Line 92 in a3005b0

  const skeletonLoader = wrapper.find('.v-skeleton-loader')

• Accessing Vue.js properties (from props/data/computed/etc):

  cylc-ui/tests/unit/components/cylc/gscan/gscan.vue.spec.js

  Line 99 in a3005b0

  expect(wrapper.vm.workflows[0].name).to.equal('five')

With that we should be able to create a small test copying the existing code that mounts the component, and gives it the necessary state for the play/stop/pause toggle.

[datamel]

This PR looks great. I understand there are tests to do and this is not maybe ready for final review. I have checked this out (in preparation for disabling unauthorised buttons) and have some observations, I think they are all appropriate to do in different PRs.

• If I create a new workflow folder, this_is_new, when picked up and selected in the ui, the stop button is correctly shown and play button enabled:
  
  However, if I select some existing workflows they have play disabled.
  
  If we want this to behave in the same way for all "Not yet run" workflows, does this need a dependency on this state?
• The icon for "Not yet run" on the left panel is the same as stopped, I think this was part of the built on pr.
  
• The stop and pause are disabled on stopping but still visible in mutation menu.
  

[oliver-sanders]

This should work for "not yet run" workflows.

The dependent cylc-flow and cylc-uiserver PRs have only just been merged so it might take a git pull.

[oliver-sanders]

Will rebase and re-test...

... looks good:

[kinow]

Code looks good to me. Worth a quick test @AaronDCole , and not sure if we already have tests or if we need some unit/e2e tests. You'll inherit this one @AaronDCole 😬

[oliver-sanders]

Testing this sort of thing without live data is going to be a bit tricky/limited, can have a go, will take a look next week.

Long term I think a system integration test suite for cylc-flow, cylc-uiserver and cylc-ui with Cypress might be a good way to go.

Would be good to slide this into the beta if possible so may need to go on manual testing for now and bump to an issue.

[MetRonnie]

Does this close #748?

[oliver-sanders]

I think #748 was closed by the related #805

[oliver-sanders]

Rebased, added changelog.

[hjoliver]

LGTM

[datamel]

I have checked out the branch and tested the change. Looks great, thanks @oliver-sanders.
The change is not covered by a test, like you said it is a hard one to test and I'm not quite sure how I would go about it so I am merging having manually tested, I could open an issue for the testing if you think it would be fitting?

[oliver-sanders]

I think we have an issue on cylc-admin for system testing.

My best idea ATM is to use Cypress to test the UI but using live data provided by a UIS.