[Question] Should we encourage hardcoded values in FE specs?
Description
Identified in this comment
thought (non-blocking): It's times like these where I wish we did not reuse constants in specs so, and just hardcoded values, so we can have some visibility if there are any value changes.
🤔
Exhibit A - Specs that use hardcoded values
GraphQL Typenames (example mr_popover_spec.js#L19)
describe('MR Popover', () => {
let wrapper;
Vue.use(VueApollo);
const mrQueryResponse = {
data: {
project: {
__typename: 'Project',
id: '1',
mergeRequest: {
__typename: 'Merge Request',
id: 'gid://gitlab/Merge_Request/1',
createdAt: '2020-07-01T04:08:01Z',
state: 'opened',
title: 'MR title',
headPipeline: {
id: '1',
detailedStatus: {
id: '1',
icon: 'status_success',
group: 'success',
},
},
},
},
},
};
I18n messages (example cc_validation_required_alert_spec.js#L53)
it('renders title', () => {
expect(findGlAlert().attributes('title')).toBe('User validation required');
});
Exhibit B - Specs that use constants
GraphQL Typenames (example utils_spec.js#L118)
expect(decomposeApproversV2([userApprover])).toStrictEqual({
[GROUP_TYPE]: [],
[USER_TYPE]: [
{
...userApprover,
type: USER_TYPE,
value: convertToGraphQLId(TYPENAME_USER, userApprover.id),
},
],
});
I18n messages (example analytics_clipboard_input_spec.js#L64)
it('shows hint when copying fails', async () => {
jest.spyOn(navigator.clipboard, 'writeText').mockRejectedValue(new Error('Failed'));
expect(findFormGroup().attributes('description')).toBe('');
findButton().vm.$emit('click');
await waitForPromises();
expect(findFormGroup().attributes('description')).toBe(i18n.failedToCopy);
expect(findTooltip().attributes('title')).toBe(i18n.copyToClipboard);
});
Thoughts
- Currently the internationalization guide recommends against hardcoding values. Since Everything-is-in-Draft, we're just wondering if this is still beneficial.
- Sometimes we refactor constants (example mr). In these situations, it's nice to have some value assertion, and not just reference assertion (i.e. did we just use the same constant twice).
- With an assertion like
expect(...).toBe(MyComponent.i18n.fooMessage)
, what ifMyComponent.i18n.fooMessage
is actually undefined? If I fill theexpect
with something likewrapper.attributes('not-a-real-attribute')
then it'll look like I'm testing something was set, when it reality it's a test that doesn't test anything.