-
Notifications
You must be signed in to change notification settings - Fork 26
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
Add Services #38
Comments
One implementation is now partially done in https://github.com/kinow/cylc-web/tree/add-services. Now need to try to write a unit test. If we are able to test everything, except the HTTP call (mocked), then I think this is a good starting point for future services used to talk to suite list service, other parts of the hub like proxy API, etc. |
Writing the unit test was quite easy and simple. The plan was to see the test failing, and use Sinon.JS to mock the call. However, faced a different issue as importing the services, apparently webpack imported the classes with GraphQL. And there is a known issue that running the Apollo GraphQL client in the node environment (i.e. without a browser), it fails due to the missing If you open a browser window, then go to any URL, open the developer tools, and type in the Console: This was fixed in Apollo apollographql/apollo-client#3578, and there is now a way to pass a |
I've moved the GraphQL call from Vuex to its own service anyway, and thought that maybe we could fix this issue later. But in someway the module is still loaded, and the whole tests fail with
So now gotta find a way to pass a fetch object before each test. But in a way that is simpler... because just asking developers to copy and paste a block of code for each test is not quite good. |
Tried a root-level hook for Mocha, but even that way the error appears to occur before Mocha loads the hook. The only way to fix it was using // import { createProvider } from './vue-apollo'
import ApolloClient from 'apollo-boost'
import fetch from 'node-fetch'
// GraphQL
const apolloClient = new ApolloClient({
// You should use an absolute URL here
uri: "https://api.graphcms.com/simple/v1/awesomeTalksClone",
// we must define fetch, as otherwise running unit tests it would fail as there is no global fetch variable
fetch: fetch
});
export default apolloClient Though that introduced a warning when running unit tests 😥
Npm node-fetch documentation says:
As it appears to work fine, leaving it for now to prevent increasing our final produced HTML/JS/CSS, etc. |
Done! We have services! And we are retrieving the User profile from the service. And this user service is retrieving the data from JupyterHub! Phew! That was quite a long ride to get the data from the hub 😪 |
Vue.js takes care of view/visualization. It means that you are free to architect the rest of your application. That is quite different from other frameworks such as React or Angular.
However, certain patterns from these other frameworks can be useful. In AngularJS, for instance, most developers are familiar with services. And when well designed, it is fairly simple to write tests mocking the services.
Without doing this way, we should still be able to create the application, run, get the right results, etc. However, writing tests without being able to mock a service, forces you to hack your API. By inserting methods that replace parts of an object.
And then eventually you end up having to spend a lot of time maintaining your main code, and also your test code. Which may become a blocker when you have to upgrade or change the framework.
This issue is to design how the components & services should play together, and then create the
UserService
for #33 . That service should be able to use Vuex actions to store the state. The only method necessary now is to retrieve the user profile (i.e. one user, for a given URL like/user/kinow/userprofile
).Not necessary, but it would be better if we could have unit tests designed too. IOW, have a test for the
UserService
, in a simple way, that can be replicated later for future services. Later we may have to update the Suite component to remove graphql dependency from Vuex.The text was updated successfully, but these errors were encountered: