Improve e2e tests of Vue code
Problem
Currently we almost don't write 2e2 tests for Vue code.
In the past we used to use rspec
to write e2e tests.
When we started using vue, because we can test the rendered output through unit tests and rspec takes a while to run, we start writing less rspec
tests for the new Vue code.
Current Status
Our Vue Code has this architecture - https://docs.gitlab.com/ce/development/fe_guide/vue.html#vue-architecture - the main component will make a call to the Service that on it's hand makes a call to the API, the smaller components receive props and are unaware of an API or service allowing us to reuse them wherever needed.
Unit tests for the smaller/dumb components
These are the easiest - just provide the possible props - we follow the Vue docs - https://vuejs.org/v2/guide/unit-testing.html#Simple-Assertions - and mostly test the rendered output and more complicated methods
or computed props
(We had a talk by Roman Kuba explaning this concept to us, where we also suggested that we would test the rendered output and more complicate methods and computedProps)
Unit tests for the main components
The main components make a call to the server, there is no way we can test them without mocking an API call.
Currently we do that with mocked data - ie, we use the data the API is sending at that moment, copy it and store it in the file called mock_data
.
Until now this allowed us not only to unit test the component:
Here's an example, the environments code has mocks for 3 possible scenarios:
- A successful request without data: https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/javascripts/environments/environment_spec.js#L19
- A successful request with data & pagination https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/javascripts/environments/environment_spec.js#L58
- A request that return 500 https://gitlab.com/gitlab-org/gitlab-ce/blob/master/spec/javascripts/environments/environment_spec.js#L165
However if the API response is changed on the backend side, all out tests will fail silently.
If we had a way to make sure the data in the mocked request was reliable and up to date with the API changes, we could only use JavaScript to write e2e tests for Vue code at the same time we write the unit ones
Dicussion
The original discussion/idea about this came from the comments in this MR: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/3215:
https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/3215#note_45678570
Kamil: @filipa My problem with leaving it like this is that we nowhere test an agreement between FE and BE on how and what data are accessed, effectively this can easily go wrong and we miss whether it works or not. It seems to me to be very fragile, and in the past, we had similar issues, because of the above.
We should at least ensure that needed data by FE are actually parsed correctly by FE. We have tests for Vue code, but we do not test the mocked responses, and effectively we have some serious hole in our tests.
If we would mock actual requests for FE, we could consider building an extra agreement
rspec
that would verify the mock provided for FE with validation of structure sent by BE.Look at this file: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/3215/diffs#359d546597979f85920f3d54a487f90c0eff0f05_215_215: I added extra data there (practically it is not ideal either, as this is not a response to request sent by FE). Can you ensure that you test the presence of that exact link in the Vue tests?
Filipa: @ayufan we are on the same page, I 100% agree with you:
If we would mock actual requests for FE, we could consider building an extra agreement rspec that would verify the mock provided for FE with validation of structure sent by BE.
We mock the requests with the supposedly correct data in the codeclimate code. The one you are referring to here
Look at this file: !3215 (merged) (diffs): I added extra data there (practically it is not ideal either, as this is not a response to request sent by FE). Can you ensure that you test the presence of that exact link in the Vue tests?
has 0 tests atm in master, regarding mocking received data, and I added them here: !3207 (merged) (diffs)
Still, these are not e2e tests, which we do need to address. I 100% agree with you, atm if a an object key name changes, all our code that consumes an API will fail silently, not just the one in this MR.
But I do think this is a little out of scope of this MR. Let's open an issue and discuss it there
💪 What do you think?
Filipa: Currently we mock the requests with dummy data, if there was a way to mock this requests with real data, that would solve the problem we are discussing.
Kamil: This seems like a good idea to me, as we can do double sword checking:
On Backend validate our mock data with API, On Frontend use our mock data.
It seems to be out of the scope of this MR, can we do it better next time? Do you see some way to make this work?
Conclusion
This could avoid losing time in manual testing & regression.
@bjgopinath what do you think?