Conversation
- Extract spotify javascript and API calls into separate module - Add tests that correct calls are made with correct arguments
| @@ -0,0 +1,60 @@ | |||
| /* global Spotify */ | |||
There was a problem hiding this comment.
Isolate API and SDK side-effects for easier testing
| const original = axios.defaults.adapter | ||
| axios.defaults.adapter = request | ||
|
|
||
| const configWith = options => ({ |
There was a problem hiding this comment.
Not a huge fan of this - axios has a handful of default request configuration options which made it difficult to do a direct assertion on what the config looked like as a request.
Now that I'm looking at this, we could just record a collection of the config objects, which would make it easier to do something like expect(config[0].url).toBe('...') rather than abuse jest mock functions.
| ) | ||
|
|
||
| this.player.addListener('ready', ({device_id}) => { | ||
| this.player.on('ready', ({device_id}) => { |
There was a problem hiding this comment.
Looks like addListener and on do the same thing, so I switch this to make my test mocking easier, and to make this block more consistent.
| } | ||
| }); | ||
| SpotifyService.shuffle(deviceId, state, this.token.token) | ||
| .catch(console.error) // eslint-disable-line no-console |
There was a problem hiding this comment.
This will need to be more robust. The main cases here are:
- No internet (status code 0?); probably show a warning/error
- Expired token (status code 401); open a popup, do the auth flow
- Spotify API is out (status code >= 400) ; maybe same flow as 1
- We introduced a bug (exception); maybe same flow as 1
We could put that logic in a service module, since there may be a number of places where we need to handle API errors.
| "@vue/cli-plugin-unit-jest": "^3.10.0", | ||
| "@vue/cli-service": "^3.8.0", | ||
| "@vue/test-utils": "^1.0.0-beta.29", | ||
| "babel-core": "7.0.0-bridge.0", |
There was a problem hiding this comment.
The vue cli added this dep for me, but I am pretty sure recent jest versions don't need this any more.
| /dist | ||
|
|
||
| # local env files | ||
| .env |
There was a problem hiding this comment.
I believe all the contents of this .gitignore are what were generated when Vue CLI created the app. Strange it didn't include .env
2 participants
Summary