Skip to content
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

Update Vuetify from 2.2.6 to 2.3.9 #466

Merged
merged 3 commits into from
Aug 20, 2020
Merged

Conversation

kinow
Copy link
Member

@kinow kinow commented Jun 19, 2020

These changes partially address #465

Updates to the latest version of Vuetify. Looked at each view of our app, with JupyterHub and five running. Everything looks OK.

Kudos to Vuetify developers. They kept backward compatibility as it's not a new major release, and deprecated the values changed, without breaking the user's code. Furthermore, they also added warning messages to each deprecated item, with instructions how to fix it 🙏

image

So after updating the version, I simply fixed as they suggested, and then no more warnings in the console.

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).
  • Does not need tests (why?).
  • No change log entry required (why? e.g. invisible to users).
  • No documentation update required.

@kinow kinow added this to the 0.2 milestone Jun 19, 2020
@kinow kinow self-assigned this Jun 19, 2020
@codecov-commenter
Copy link

codecov-commenter commented Jun 19, 2020

Codecov Report

Merging #466 into master will increase coverage by 12.27%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #466       +/-   ##
===========================================
+ Coverage   57.07%   69.35%   +12.27%     
===========================================
  Files          53       57        +4     
  Lines        1046     1266      +220     
  Branches       75       79        +4     
===========================================
+ Hits          597      878      +281     
+ Misses        435      367       -68     
- Partials       14       21        +7     
Flag Coverage Δ
#e2e 48.53% <ø> (+5.29%) ⬆️
#unittests 52.64% <ø> (+6.42%) ⬆️

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

Impacted Files Coverage Δ
src/components/cylc/Drawer.vue 75.00% <ø> (ø)
src/components/cylc/graph/Graph.vue 24.52% <ø> (+24.52%) ⬆️
src/services/mock/workflow.service.mock.js 87.50% <0.00%> (-12.50%) ⬇️
src/services/gquery.js 77.41% <0.00%> (-6.46%) ⬇️
src/graphql/queries.js 100.00% <0.00%> (ø)
src/store/workflows.module.js 100.00% <0.00%> (ø)
src/services/workflow.service.js 0.00% <0.00%> (ø)
src/views/Tree.vue 66.66% <0.00%> (ø)
src/components/cylc/tree/deltas.js 100.00% <0.00%> (ø)
src/mixins/treeview.js 100.00% <0.00%> (ø)
... and 12 more

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 4992128...7642c1d. Read the comment docs.

@kinow
Copy link
Member Author

kinow commented Jun 19, 2020

Really no idea how to fix the 2 tests failing. Managed to fix one, with a lot of changes to Vuex, which is not a good sign. Probably something in the new version that changed requires us to change something simple in the way we create the object for testing... hopefully.

@kinow
Copy link
Member Author

kinow commented Jun 21, 2020

Failing test should be fixed now. Lessons learned:

  • We were using mapState directly in the component, which is fine, but means that writing a unit test for the component requires you to properly handle Vuex in the test too. Our store has many entries already, even though it's far from comparing to large applications. Which means it's harder to maintain tests and make sure everything works.
  • By moving the mapState to the caller of the component, the unit tests gets much simpler. All you need to pass to the component is a prop. Testing components with props in Mocha+Chai is easy-peasy.
  • I thought the connection-status component was also hidden, but turns out now the component is present, but the SnackBar component is hidden; I guess Vuetify has some reason for doing it that way
  • Mocha API for parameterized tests is documented under their "Dynamically Generating Tests" docs section. Not as nice as in other test frameworks, but it works well.
  • The output of failing tests is not really helpful if you have similar tests, meaning that you have to scroll up-down to find the line of the error, and it may still not match due to sourcemap... so you need to locate the function name. A helpful tip is to add a message to the last function in the assertion (e.g. expect(predicate).to.be(true, 'This is a helpful message').

@kinow
Copy link
Member Author

kinow commented Jun 21, 2020

Set to 0.2, but can be merged later and included in 0.3 too, no hurry on this one 👍

Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kinow kinow modified the milestones: 0.2, 0.3 Jul 14, 2020
@kinow
Copy link
Member Author

kinow commented Jul 14, 2020

Pushed to 0.3. Will rebase it later.

@kinow
Copy link
Member Author

kinow commented Aug 18, 2020

2.3.9 is out, but we can update that later. Should be much easier than upgrading 2.2 to 2.3.

kinow added a commit to kinow/cylc-ui that referenced this pull request Aug 19, 2020
I used `ncu` to get a list of dependencies that had updates available. Then went through these dependencies individually, updating each one and checking if the build was working OK.

`vuetify` was not updated as it is being updated in cylc#466.

Updating `eslint-config-vuetify` resulted in some weird errors in `yarn install`:

error An unexpected error occurred: "could not find a copy of eslint to link in /home/kinow/Development/python/workspace/cylc-ui/node_modules/eslint-config-vuetify/node_modules".

So I reverted it and will take care of that dependency later (it is a dev dependency).
@kinow kinow mentioned this pull request Aug 19, 2020
6 tasks
@kinow kinow changed the title Update Vuetify from 2.2.6 to 2.3.1 Update Vuetify from 2.2.6 to 2.3.9 Aug 19, 2020
@kinow
Copy link
Member Author

kinow commented Aug 19, 2020

No issues updating to 2.3.9, so pushed the changes to this PR in my last commit. Tested some views, and found nothing wrong with how components were displayed 👍 also no build errors.

kinow added a commit to kinow/cylc-ui that referenced this pull request Aug 19, 2020
I used `ncu` to get a list of dependencies that had updates available. Then went through these dependencies individually, updating each one and checking if the build was working OK.

`vuetify` was not updated as it is being updated in cylc#466.

Updating `eslint-config-vuetify` resulted in some weird errors in `yarn install`:

error An unexpected error occurred: "could not find a copy of eslint to link in /home/kinow/Development/python/workspace/cylc-ui/node_modules/eslint-config-vuetify/node_modules".

So I reverted it and will take care of that dependency later (it is a dev dependency).
Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested as working, all seems good.

@oliver-sanders oliver-sanders merged commit 92db6fd into cylc:master Aug 20, 2020
@kinow kinow deleted the update-vuetify branch August 20, 2020 08:50
kinow added a commit to kinow/cylc-ui that referenced this pull request Aug 20, 2020
I used `ncu` to get a list of dependencies that had updates available. Then went through these dependencies individually, updating each one and checking if the build was working OK.

`vuetify` was not updated as it is being updated in cylc#466.

Updating `eslint-config-vuetify` resulted in some weird errors in `yarn install`:

error An unexpected error occurred: "could not find a copy of eslint to link in /home/kinow/Development/python/workspace/cylc-ui/node_modules/eslint-config-vuetify/node_modules".

So I reverted it and will take care of that dependency later (it is a dev dependency).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants