From 31b297099e42af0a4a45aab144a408d416f4a34b Mon Sep 17 00:00:00 2001 From: Miguel Rincon Date: Fri, 4 Dec 2020 15:18:26 +0100 Subject: [PATCH 1/6] Add static validation to gitlab CI yaml This change fetches the external YML schema for the gitlab-ci file and adds it to the Pipeline/CI Editor. --- .../editor/editor_ci_schema_ext.js | 37 ++++++++ .../components/text_editor.vue | 33 +++++++- .../pipeline_editor/pipeline_editor_app.vue | 8 +- .../vue_shared/components/editor_lite.vue | 2 +- .../editor/editor_ci_schema_ext_spec.js | 84 +++++++++++++++++++ .../components/text_editor_spec.js | 53 ++++++++++-- .../pipeline_editor_app_spec.js | 27 ++++-- .../vue_shared/components/editor_lite_spec.js | 13 ++- 8 files changed, 240 insertions(+), 17 deletions(-) create mode 100644 app/assets/javascripts/editor/editor_ci_schema_ext.js create mode 100644 spec/frontend/editor/editor_ci_schema_ext_spec.js diff --git a/app/assets/javascripts/editor/editor_ci_schema_ext.js b/app/assets/javascripts/editor/editor_ci_schema_ext.js new file mode 100644 index 00000000000000..454666b6263cac --- /dev/null +++ b/app/assets/javascripts/editor/editor_ci_schema_ext.js @@ -0,0 +1,37 @@ +import { registerSchema } from '~/ide/utils'; +import Api from '~/api'; + +/** + * Gets the URI of CI config JSON schema file + */ +const getCiSchemaUri = ({ projectPath, ref }) => { + // Note: This `:filename` is hardcoded regardless + // project configuration, see more: + // - app/services/ide/schemas_config_service.rb + const SCHEMA_FILE_NAME_MATCH = '.gitlab-ci.yml'; + const [namespace, project] = projectPath.split('/'); + + return Api.buildUrl(Api.projectFileSchemaPath) + .replace(':namespace_path', namespace) + .replace(':project_path', project) + .replace(':ref', ref) + .replace(':filename', SCHEMA_FILE_NAME_MATCH); +}; + +export default { + /** + * Registers a schema in a model based on project properties + * and the name of the file that is edited. + * + * @param {Object} opts + * @param {String} opts.projectPath + * @param {String} opts.ref + * @param {String} opts.fileName - CI config file name, should match the "file-name" used in the editor. + */ + registerCiSchema({ projectPath, ref = 'master', fileName = '.gitlab-ci.yml' }) { + registerSchema({ + uri: getCiSchemaUri({ projectPath, ref }), + fileMatch: [fileName], + }); + }, +}; diff --git a/app/assets/javascripts/pipeline_editor/components/text_editor.vue b/app/assets/javascripts/pipeline_editor/components/text_editor.vue index 22f2a32c9ac03d..d1152b61384215 100644 --- a/app/assets/javascripts/pipeline_editor/components/text_editor.vue +++ b/app/assets/javascripts/pipeline_editor/components/text_editor.vue @@ -1,14 +1,45 @@ diff --git a/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue b/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue index b1c52ffa9205f4..95e4715a274327 100644 --- a/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue +++ b/app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue @@ -258,7 +258,13 @@ export default { - + diff --git a/app/assets/javascripts/vue_shared/components/editor_lite.vue b/app/assets/javascripts/vue_shared/components/editor_lite.vue index cfe3ce0a11cab1..f2bf5895fb3977 100644 --- a/app/assets/javascripts/vue_shared/components/editor_lite.vue +++ b/app/assets/javascripts/vue_shared/components/editor_lite.vue @@ -92,7 +92,7 @@ export default { :id="`editor-lite-${fileGlobalId}`" ref="editor" data-editor-loading - @editor-ready="$emit('editor-ready')" + @editor-ready="$emit('editor-ready', editor)" > 
{{ value }}
diff --git a/spec/frontend/editor/editor_ci_schema_ext_spec.js b/spec/frontend/editor/editor_ci_schema_ext_spec.js new file mode 100644 index 00000000000000..b1c36adb16d4a0 --- /dev/null +++ b/spec/frontend/editor/editor_ci_schema_ext_spec.js @@ -0,0 +1,84 @@ +import { languages } from 'monaco-editor'; +import EditorLite from '~/editor/editor_lite'; +import EditorCiSchemaExtension from '~/editor/editor_ci_schema_ext'; + +describe('~/editor/editor_ci_config_ext', () => { + let editor; + let instance; + let editorEl; + + beforeEach(() => { + setFixtures('
'); + editorEl = document.getElementById('editor'); + editor = new EditorLite(); + instance = editor.createInstance({ + el: editorEl, + blobPath: '', + blobContent: '', + }); + editor.use(EditorCiSchemaExtension); + }); + + afterEach(() => { + instance.dispose(); + editorEl.remove(); + }); + + describe('registerCiSchema', () => { + beforeEach(() => { + jest.spyOn(languages.json.jsonDefaults, 'setDiagnosticsOptions'); + jest.spyOn(languages.yaml.yamlDefaults, 'setDiagnosticsOptions'); + }); + + describe('register validations options with monaco for both json and yaml', () => { + it('with expected basic validation configuration', () => { + instance.registerCiSchema({ projectPath: 'namespace/my-project' }); + + const expectedOptions = { + validate: true, + enableSchemaRequest: true, + hover: true, + completion: true, + }; + + expect(languages.json.jsonDefaults.setDiagnosticsOptions).toHaveBeenCalledWith( + expect.objectContaining(expectedOptions), + ); + expect(languages.yaml.yamlDefaults.setDiagnosticsOptions).toHaveBeenCalledWith( + expect.objectContaining(expectedOptions), + ); + }); + + it.each` + opts | expectedFileMatch | expectedUri + ${{}} | ${'.gitlab-ci.yml'} | ${'/namespace/my-project/-/schema/master/.gitlab-ci.yml'} + ${{ ref: 'REF' }} | ${'.gitlab-ci.yml'} | ${'/namespace/my-project/-/schema/REF/.gitlab-ci.yml'} + ${{ fileName: 'custom-ci.yml' }} | ${'custom-ci.yml'} | ${'/namespace/my-project/-/schema/master/.gitlab-ci.yml'} + `( + 'with the expected schema for options "$opts"', + ({ opts, expectedUri, expectedFileMatch }) => { + instance.registerCiSchema({ + projectPath: 'namespace/my-project', + ...opts, + }); + + const expectedOptions = expect.objectContaining({ + schemas: [ + { + uri: expectedUri, + fileMatch: [expectedFileMatch], + }, + ], + }); + + expect(languages.json.jsonDefaults.setDiagnosticsOptions).toHaveBeenCalledWith( + expectedOptions, + ); + expect(languages.yaml.yamlDefaults.setDiagnosticsOptions).toHaveBeenCalledWith( + expectedOptions, + ); + }, + ); + }); + }); +}); diff --git a/spec/frontend/pipeline_editor/components/text_editor_spec.js b/spec/frontend/pipeline_editor/components/text_editor_spec.js index 18f71ebc95cc08..9d51468a02b743 100644 --- a/spec/frontend/pipeline_editor/components/text_editor_spec.js +++ b/spec/frontend/pipeline_editor/components/text_editor_spec.js @@ -1,15 +1,27 @@ import { shallowMount } from '@vue/test-utils'; import EditorLite from '~/vue_shared/components/editor_lite.vue'; -import { mockCiYml } from '../mock_data'; - +import EditorCiSchemaExtension from '~/editor/editor_ci_schema_ext'; import TextEditor from '~/pipeline_editor/components/text_editor.vue'; +import { mockCiYml, mockCiConfigPath, mockProjectPath } from '../mock_data'; + describe('~/pipeline_editor/components/text_editor.vue', () => { let wrapper; + let editorInstance; const editorReadyListener = jest.fn(); - const createComponent = (attrs = {}, mountFn = shallowMount) => { + const createComponent = ({ props = {}, attrs = {} } = {}, mountFn = shallowMount) => { + editorInstance = { + use: jest.fn(), + registerCiSchema: jest.fn(), + }; + wrapper = mountFn(TextEditor, { + propsData: { + ciConfigPath: mockCiConfigPath, + projectPath: mockProjectPath, + ...props, + }, attrs: { value: mockCiYml, ...attrs, @@ -20,11 +32,18 @@ describe('~/pipeline_editor/components/text_editor.vue', () => { }); }; + beforeEach(() => { + createComponent(); + }); + + afterEach(() => { + wrapper.destroy(); + wrapper = null; + }); + const findEditor = () => wrapper.find(EditorLite); it('contains an editor', () => { - createComponent(); - expect(findEditor().exists()).toBe(true); }); @@ -33,12 +52,32 @@ describe('~/pipeline_editor/components/text_editor.vue', () => { }); it('editor is configured for .yml', () => { - expect(findEditor().props('fileName')).toBe('*.yml'); + expect(findEditor().props('fileName')).toBe(mockCiConfigPath); }); it('bubbles up events', () => { - findEditor().vm.$emit('editor-ready'); + findEditor().vm.$emit('editor-ready', editorInstance); expect(editorReadyListener).toHaveBeenCalled(); }); + + it('registers ci schema extension', () => { + const mockRef = 'master'; + + createComponent({ + props: { + commitId: mockRef, + }, + }); + + findEditor().vm.$emit('editor-ready', editorInstance); + + expect(editorInstance.use).toHaveBeenCalledWith(EditorCiSchemaExtension); + + expect(editorInstance.registerCiSchema).toHaveBeenCalledWith({ + fileName: mockCiConfigPath, + projectPath: mockProjectPath, + ref: mockRef, + }); + }); }); diff --git a/spec/frontend/pipeline_editor/pipeline_editor_app_spec.js b/spec/frontend/pipeline_editor/pipeline_editor_app_spec.js index ca54c97d2bb87e..50f6c4e8d3823a 100644 --- a/spec/frontend/pipeline_editor/pipeline_editor_app_spec.js +++ b/spec/frontend/pipeline_editor/pipeline_editor_app_spec.js @@ -34,6 +34,10 @@ import TextEditor from '~/pipeline_editor/components/text_editor.vue'; const localVue = createLocalVue(); localVue.use(VueApollo); +const MockEditorLite = { + template: '
', +}; + jest.mock('~/lib/utils/url_utility', () => ({ redirectTo: jest.fn(), refreshCurrentPage: jest.fn(), @@ -77,9 +81,7 @@ describe('~/pipeline_editor/pipeline_editor_app.vue', () => { GlTabs, GlButton, CommitForm, - EditorLite: { - template: '
', - }, + EditorLite: MockEditorLite, TextEditor, }, mocks: { @@ -125,7 +127,7 @@ describe('~/pipeline_editor/pipeline_editor_app.vue', () => { const findLoadingIcon = () => wrapper.find(GlLoadingIcon); const findAlert = () => wrapper.find(GlAlert); const findTabAt = i => wrapper.findAll(GlTab).at(i); - const findTextEditor = () => wrapper.find(TextEditor); + const findTextEditor = () => wrapper.find(MockEditorLite); const findCommitForm = () => wrapper.find(CommitForm); const findCommitBtnLoadingIcon = () => wrapper.find('[type="submit"]').find(GlLoadingIcon); @@ -173,7 +175,7 @@ describe('~/pipeline_editor/pipeline_editor_app.vue', () => { it('displays editor tab lazily, until editor is ready', async () => { expect(findTabAt(0).attributes('lazy')).toBe('true'); - findTextEditor().vm.$emit('editor-ready'); + findTextEditor().vm.$emit('editor-ready', { use: jest.fn(), registerCiSchema: jest.fn() }); await nextTick(); @@ -192,8 +194,23 @@ describe('~/pipeline_editor/pipeline_editor_app.vue', () => { }); it('displays content after the query loads', () => { + const mockRegisterCiSchema = jest.fn(); + + findTextEditor().vm.$emit('editor-ready', { + use: jest.fn(), + registerCiSchema: mockRegisterCiSchema, + }); + expect(findLoadingIcon().exists()).toBe(false); + + expect(findTextEditor().attributes('file-name')).toBe(mockCiConfigPath); expect(findTextEditor().attributes('value')).toBe(mockCiYml); + + expect(mockRegisterCiSchema).toHaveBeenCalledWith({ + fileName: mockCiConfigPath, + projectPath: mockProjectPath, + ref: mockCommitId, + }); }); describe('commit form', () => { diff --git a/spec/frontend/vue_shared/components/editor_lite_spec.js b/spec/frontend/vue_shared/components/editor_lite_spec.js index 52502fcf64fc55..3ebdfba85440bb 100644 --- a/spec/frontend/vue_shared/components/editor_lite_spec.js +++ b/spec/frontend/vue_shared/components/editor_lite_spec.js @@ -108,13 +108,22 @@ describe('Editor Lite component', () => { expect(wrapper.emitted().input).toEqual([[value]]); }); - it('emits editor-ready event when the Editor Lite is ready', async () => { + it('emits editor-ready event with the editor instance when the Editor Lite is ready', async () => { const el = wrapper.find({ ref: 'editor' }).element; expect(wrapper.emitted()['editor-ready']).toBeUndefined(); await el.dispatchEvent(new Event('editor-ready')); - expect(wrapper.emitted()['editor-ready']).toBeDefined(); + expect(wrapper.emitted()['editor-ready']).toHaveLength(1); + expect(wrapper.emitted()['editor-ready'][0]).toEqual([ + expect.objectContaining({ + onDidChangeModelContent: expect.any(Function), + updateModelLanguage: expect.any(Function), + getValue: expect.any(Function), + setValue: expect.any(Function), + dispose: expect.any(Function), + }), + ]); }); describe('reaction to the value update', () => { -- GitLab From a83d0af5dd0a43954399e2248f7ce7db5c25d083 Mon Sep 17 00:00:00 2001 From: Miguel Rincon Date: Wed, 9 Dec 2020 16:09:38 +0100 Subject: [PATCH 2/6] Add changes from review - Removed editor instance from event - Added tests for checking integrations between components --- .../editor/editor_ci_schema_ext.js | 11 ++-- .../components/text_editor.vue | 5 +- .../vue_shared/components/editor_lite.vue | 5 +- .../editor/editor_ci_schema_ext_spec.js | 59 ++++++++----------- .../components/text_editor_spec.js | 47 ++++++--------- .../pipeline_editor_app_spec.js | 49 +++++++-------- .../vue_shared/components/editor_lite_spec.js | 51 ++++++++-------- 7 files changed, 102 insertions(+), 125 deletions(-) diff --git a/app/assets/javascripts/editor/editor_ci_schema_ext.js b/app/assets/javascripts/editor/editor_ci_schema_ext.js index 454666b6263cac..0133349a3dca9e 100644 --- a/app/assets/javascripts/editor/editor_ci_schema_ext.js +++ b/app/assets/javascripts/editor/editor_ci_schema_ext.js @@ -24,11 +24,14 @@ export default { * and the name of the file that is edited. * * @param {Object} opts - * @param {String} opts.projectPath - * @param {String} opts.ref - * @param {String} opts.fileName - CI config file name, should match the "file-name" used in the editor. + * @param {String} opts.projectPath - Namespace and project in the form `namespace/project` + * @param {String?} opts.ref */ - registerCiSchema({ projectPath, ref = 'master', fileName = '.gitlab-ci.yml' }) { + registerCiSchema({ projectPath, ref = 'master' }) { + // TODO Check syntax + const fileName = this.getModel() + .uri.path.split('/') + .pop(); registerSchema({ uri: getCiSchemaUri({ projectPath, ref }), fileMatch: [fileName], diff --git a/app/assets/javascripts/pipeline_editor/components/text_editor.vue b/app/assets/javascripts/pipeline_editor/components/text_editor.vue index d1152b61384215..d21e86ec726727 100644 --- a/app/assets/javascripts/pipeline_editor/components/text_editor.vue +++ b/app/assets/javascripts/pipeline_editor/components/text_editor.vue @@ -22,10 +22,10 @@ export default { }, }, methods: { - onEditorReady(editorInstance) { + onEditorReady() { + const editorInstance = this.$refs.editor.getEditor(); editorInstance.use(EditorCiSchemaExtension); editorInstance.registerCiSchema({ - fileName: this.ciConfigPath, projectPath: this.projectPath, ref: this.commitId, }); @@ -36,6 +36,7 @@ export default {