Merge branch '263147-ci-cd-home-mvc-ci-linter-2' into 'master'

Add linting tab to the CI editor

See merge request gitlab-org/gitlab!48901

app/assets/javascripts/pipeline_editor/components/lint/ci_lint.vue

+<script>
+import { CI_CONFIG_STATUS_VALID } from '../../constants';
+import CiLintResults from './ci_lint_results.vue';
+
+export default {
+  components: {
+    CiLintResults,
+  },
+  inject: {
+    lintHelpPagePath: {
+      default: '',
+    },
+  },
+  props: {
+    ciConfig: {
+      type: Object,
+      required: true,
+    },
+  },
+  computed: {
+    isValid() {
+      return this.ciConfig?.status === CI_CONFIG_STATUS_VALID;
+    },
+    stages() {
+      return this.ciConfig?.stages || [];
+    },
+    jobs() {
+      return this.stages.reduce((acc, { groups, name: stageName }) => {
+        return acc.concat(
+          groups.map(({ name: groupName }) => ({
+            stage: stageName,
+            name: groupName,
+          })),
+        );
+      }, []);
+    },
+  },
+};
+</script>
+
+<template>
+  <ci-lint-results
+    :valid="isValid"
+    :jobs="jobs"
+    :errors="ciConfig.errors"
+    :lint-help-page-path="lintHelpPagePath"
+  />
+</template>

app/assets/javascripts/pipeline_editor/components/lint/ci_lint_results.vue

@@ -10,11 +10,11 @@ const thBorderColor = 'gl-border-gray-100!';
 export default {
   correct: {
     variant: 'success',
-    text: __('syntax is correct.'),
+    text: __('Syntax is correct.'),
   },
   incorrect: {
     variant: 'danger',
-    text: __('syntax is incorrect.'),
+    text: __('Syntax is incorrect.'),
   },
   includesText: __(
     'CI configuration validated, including all configuration added with the %{codeStart}includes%{codeEnd} keyword. %{link}',
@@ -48,19 +48,23 @@ export default {
     },
     jobs: {
       type: Array,
-      required: true,
+      required: false,
+      default: () => [],
     },
     errors: {
       type: Array,
-      required: true,
+      required: false,
+      default: () => [],
     },
     warnings: {
       type: Array,
-      required: true,
+      required: false,
+      default: () => [],
     },
     dryRun: {
       type: Boolean,
-      required: true,
+      required: false,
+      default: false,
     },
     lintHelpPagePath: {
       type: String,

app/assets/javascripts/pipeline_editor/components/lint/ci_lint_results_value.vue

@@ -14,7 +14,7 @@ export default {
   },
   computed: {
     tagList() {
-      return this.item.tagList.join(', ');
+      return this.item.tagList?.join(', ');
     },
     onlyPolicy() {
       return this.item.only ? this.item.only.refs.join(', ') : this.item.only;
@@ -26,15 +26,15 @@ export default {
       return {
         beforeScript: {
           show: !isEmpty(this.item.beforeScript),
-          content: this.item.beforeScript.join('\n'),
+          content: this.item.beforeScript?.join('\n'),
         },
         script: {
           show: !isEmpty(this.item.script),
-          content: this.item.script.join('\n'),
+          content: this.item.script?.join('\n'),
         },
         afterScript: {
           show: !isEmpty(this.item.afterScript),
-          content: this.item.afterScript.join('\n'),
+          content: this.item.afterScript?.join('\n'),
         },
       };
     },
@@ -43,7 +43,7 @@ export default {
 </script>
 
 <template>
-  <div>
+  <div data-testid="ci-lint-value">
     <pre v-if="scripts.beforeScript.show" data-testid="ci-lint-before-script">{{
       scripts.beforeScript.content
     }}</pre>
@@ -53,25 +53,25 @@ export default {
     }}</pre>
 
     <ul class="gl-list-style-none gl-pl-0 gl-mb-0">
-      <li>
+      <li v-if="tagList">
         <b>{{ __('Tag list:') }}</b>
         {{ tagList }}
       </li>
       <div v-if="!dryRun" data-testid="ci-lint-only-except">
-        <li>
+        <li v-if="onlyPolicy">
           <b>{{ __('Only policy:') }}</b>
           {{ onlyPolicy }}
         </li>
-        <li>
+        <li v-if="exceptPolicy">
           <b>{{ __('Except policy:') }}</b>
           {{ exceptPolicy }}
         </li>
       </div>
-      <li>
+      <li v-if="item.environment">
         <b>{{ __('Environment:') }}</b>
         {{ item.environment }}
       </li>
-      <li>
+      <li v-if="item.when">
         <b>{{ __('When:') }}</b>
         {{ item.when }}
         <b v-if="item.allowFailure">{{ __('Allowed to fail') }}</b>

app/assets/javascripts/pipeline_editor/index.js

@@ -14,7 +14,14 @@ export const initPipelineEditor = (selector = '#js-pipeline-editor') => {
     return null;
   }
 
-  const { ciConfigPath, commitSha, defaultBranch, newMergeRequestPath, projectPath } = el?.dataset;
+  const {
+    ciConfigPath,
+    commitSha,
+    defaultBranch,
+    newMergeRequestPath,
+    lintHelpPagePath,
+    projectPath,
+  } = el?.dataset;
 
   Vue.use(VueApollo);
 
@@ -25,6 +32,9 @@ export const initPipelineEditor = (selector = '#js-pipeline-editor') => {
   return new Vue({
     el,
     apolloProvider,
+    provide: {
+      lintHelpPagePath,
+    },
     render(h) {
       return h(PipelineEditorApp, {
         props: {

app/assets/javascripts/pipeline_editor/pipeline_editor_app.vue

@@ -5,6 +5,7 @@ import { mergeUrlParams, redirectTo, refreshCurrentPage } from '~/lib/utils/url_
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 
 import PipelineGraph from '~/pipelines/components/pipeline_graph/pipeline_graph.vue';
+import CiLint from './components/lint/ci_lint.vue';
 import CommitForm from './components/commit/commit_form.vue';
 import TextEditor from './components/text_editor.vue';
 
@@ -25,6 +26,7 @@ const LOAD_FAILURE_UNKNOWN = 'LOAD_FAILURE_UNKNOWN';
 export default {
   components: {
     CommitForm,
+    CiLint,
     GlAlert,
     GlLoadingIcon,
     GlTab,
@@ -118,7 +120,7 @@ export default {
     isBlobContentLoading() {
       return this.$apollo.queries.content.loading;
     },
-    isVisualizationTabLoading() {
+    isCiConfigDataLoading() {
       return this.$apollo.queries.ciConfigData.loading;
     },
     isVisualizeTabActive() {
@@ -161,6 +163,7 @@ export default {
     defaultCommitMessage: __('Update %{sourcePath} file'),
     tabEdit: s__('Pipelines|Write pipeline configuration'),
     tabGraph: s__('Pipelines|Visualize'),
+    tabLint: s__('Pipelines|Lint'),
   },
   errorTexts: {
     [LOAD_FAILURE_NO_REF]: s__(
@@ -283,9 +286,14 @@ export default {
             :lazy="!isVisualizeTabActive"
             data-testid="visualization-tab"
           >
-            <gl-loading-icon v-if="isVisualizationTabLoading" size="lg" class="gl-m-3" />
+            <gl-loading-icon v-if="isCiConfigDataLoading" size="lg" class="gl-m-3" />
             <pipeline-graph v-else :pipeline-data="ciConfigData" />
           </gl-tab>
+
+          <gl-tab :title="$options.i18n.tabLint">
+            <gl-loading-icon v-if="isCiConfigDataLoading" size="lg" class="gl-m-3" />
+            <ci-lint v-else :ci-config="ciConfigData" />
+          </gl-tab>
         </gl-tabs>
       </div>
       <commit-form

app/views/projects/ci/pipeline_editor/show.html.haml

@@ -5,4 +5,5 @@
   "default-branch" => @project.default_branch,
   "commit-sha" => @project.commit ? @project.commit.sha : '',
   "new-merge-request-path" => namespace_project_new_merge_request_path,
+  "lint-help-page-path" => help_page_path('ci/lint', anchor: 'validate-basic-logic-and-syntax'),
 } }

locale/gitlab.pot

@@ -20485,6 +20485,9 @@ msgstr ""
 msgid "Pipelines|Last Used"
 msgstr ""
 
+msgid "Pipelines|Lint"
+msgstr ""
+
 msgid "Pipelines|Loading Pipelines"
 msgstr ""
 
@@ -27038,6 +27041,12 @@ msgstr ""
 msgid "Syncing…"
 msgstr ""
 
+msgid "Syntax is correct."
+msgstr ""
+
+msgid "Syntax is incorrect."
+msgstr ""
+
 msgid "System"
 msgstr ""
 
@@ -33810,12 +33819,6 @@ msgstr ""
 msgid "suggestPipeline|We’re adding a GitLab CI configuration file to add a pipeline to the project. You could create it manually, but we recommend that you start with a GitLab template that works out of the box."
 msgstr ""
 
-msgid "syntax is correct."
-msgstr ""
-
-msgid "syntax is incorrect."
-msgstr ""
-
 msgid "tag name"
 msgstr ""
 

spec/features/projects/ci/lint_spec.rb

@@ -34,7 +34,7 @@ RSpec.describe 'CI Lint', :js do
         end
 
         it 'parses Yaml and displays the jobs' do
-          expect(page).to have_content('Status: syntax is correct')
+          expect(page).to have_content('Status: Syntax is correct')
 
           within "table" do
             aggregate_failures do
@@ -51,7 +51,7 @@ RSpec.describe 'CI Lint', :js do
         let(:yaml_content) { 'value: cannot have :' }
 
         it 'displays information about an error' do
-          expect(page).to have_content('Status: syntax is incorrect')
+          expect(page).to have_content('Status: Syntax is incorrect')
           expect(page).to have_selector(content_selector, text: yaml_content)
         end
       end

spec/frontend/pipeline_editor/components/lint/ci_lint_results_spec.js

@@ -33,6 +33,7 @@ describe('CI Lint Results', () => {
   const findStatus = findByTestId('status');
   const findOnlyExcept = findByTestId('only-except');
   const findLintParameters = findAllByTestId('parameter');
+  const findLintValues = findAllByTestId('value');
   const findBeforeScripts = findAllByTestId('before-script');
   const findScripts = findAllByTestId('script');
   const findAfterScripts = findAllByTestId('after-script');
@@ -43,6 +44,31 @@ describe('CI Lint Results', () => {
     wrapper = null;
   });
 
+  describe('Empty results', () => {
+    it('renders with no jobs, errors or warnings defined', () => {
+      createComponent({ jobs: undefined, errors: undefined, warnings: undefined }, shallowMount);
+      expect(findTable().exists()).toBe(true);
+    });
+
+    it('renders when job has no properties defined', () => {
+      // job with no attributes such as `tagList` or `environment`
+      const job = {
+        stage: 'Stage Name',
+        name: 'test job',
+      };
+      createComponent({ jobs: [job] }, mount);
+
+      const param = findLintParameters().at(0);
+      const value = findLintValues().at(0);
+
+      expect(param.text()).toBe(`${job.stage} Job - ${job.name}`);
+
+      // This test should be updated once properties of each job are shown
+      // See https://gitlab.com/gitlab-org/gitlab/-/issues/291031
+      expect(value.text()).toBe('');
+    });
+  });
+
   describe('Invalid results', () => {
     beforeEach(() => {
       createComponent({ valid: false, errors: mockErrors, warnings: mockWarnings }, mount);

spec/frontend/pipeline_editor/components/lint/ci_lint_spec.js

+import { shallowMount, mount } from '@vue/test-utils';
+import { GlAlert, GlLink } from '@gitlab/ui';
+import CiLint from '~/pipeline_editor/components/lint/ci_lint.vue';
+import { CI_CONFIG_STATUS_INVALID } from '~/pipeline_editor/constants';
+import { mockCiConfigQueryResponse, mockLintHelpPagePath } from '../../mock_data';
+import { unwrapStagesWithNeeds } from '~/pipelines/components/unwrapping_utils';
+
+const getCiConfig = mergedConfig => {
+  const { ciConfig } = mockCiConfigQueryResponse.data;
+  return {
+    ...ciConfig,
+    stages: unwrapStagesWithNeeds(ciConfig.stages.nodes),
+    ...mergedConfig,
+  };
+};
+
+describe('~/pipeline_editor/components/lint/ci_lint.vue', () => {
+  let wrapper;
+
+  const createComponent = (props = {}, mountFn = shallowMount) => {
+    wrapper = mountFn(CiLint, {
+      provide: {
+        lintHelpPagePath: mockLintHelpPagePath,
+      },
+      propsData: {
+        ciConfig: getCiConfig(),
+        ...props,
+      },
+    });
+  };
+
+  const findAllByTestId = selector => wrapper.findAll(`[data-testid="${selector}"]`);
+  const findAlert = () => wrapper.find(GlAlert);
+  const findLintParameters = () => findAllByTestId('ci-lint-parameter');
+  const findLintParameterAt = i => findLintParameters().at(i);
+
+  afterEach(() => {
+    wrapper.destroy();
+    wrapper = null;
+  });
+
+  describe('Valid Results', () => {
+    beforeEach(() => {
+      createComponent({}, mount);
+    });
+
+    it('displays valid results', () => {
+      expect(findAlert().text()).toMatch('Status: Syntax is correct.');
+    });
+
+    it('displays link to the right help page', () => {
+      expect(
+        findAlert()
+          .find(GlLink)
+          .attributes('href'),
+      ).toBe(mockLintHelpPagePath);
+    });
+
+    it('displays jobs', () => {
+      expect(findLintParameters()).toHaveLength(3);
+
+      expect(findLintParameterAt(0).text()).toBe('Test Job - job_test_1');
+      expect(findLintParameterAt(1).text()).toBe('Test Job - job_test_2');
+      expect(findLintParameterAt(2).text()).toBe('Build Job - job_build');
+    });
+
+    it('displays invalid results', () => {
+      createComponent(
+        {
+          ciConfig: getCiConfig({
+            status: CI_CONFIG_STATUS_INVALID,
+          }),
+        },
+        mount,
+      );
+
+      expect(findAlert().text()).toMatch('Status: Syntax is incorrect.');
+    });
+  });
+});

spec/frontend/pipeline_editor/mock_data.js

+import { CI_CONFIG_STATUS_VALID } from '~/pipeline_editor/constants';
+
 export const mockProjectPath = 'user1/project1';
 export const mockDefaultBranch = 'master';
 export const mockNewMergeRequestPath = '/-/merge_requests/new';
 export const mockCommitSha = 'aabbccdd';
 export const mockCommitNextSha = 'eeffgghh';
+export const mockLintHelpPagePath = '/-/lint-help';
 export const mockCommitMessage = 'My commit message';
 
 export const mockCiConfigPath = '.gitlab-ci.yml';
 export const mockCiYml = `
-job1:
+stages:
+  - test
+  - build
+
+job_test_1:
   stage: test
+  script:
+    - echo "test 1"
+
+job_test_2:
+  stage: test
+  script:
+    - echo "test 2"
+
+job_build:
+  stage: build
   script: 
-    - echo 'test'
+    - echo "build"
+  needs: ["job_test_2"]
 `;
 
 export const mockCiConfigQueryResponse = {
   data: {
     ciConfig: {
       errors: [],
-      stages: [],
-      status: '',
+      status: CI_CONFIG_STATUS_VALID,
+      stages: {
+        __typename: 'CiConfigStageConnection',
+        nodes: [
+          {
+            name: 'test',
+            groups: {
+              nodes: [
+                {
+                  name: 'job_test_1',
+                  jobs: {
+                    nodes: [
+                      {
+                        name: 'job_test_1',
+                        needs: { nodes: [], __typename: 'CiConfigNeedConnection' },
+                        __typename: 'CiConfigJob',
+                      },
+                    ],
+                    __typename: 'CiConfigJobConnection',
+                  },
+                  __typename: 'CiConfigGroup',
+                },
+                {
+                  name: 'job_test_2',
+                  jobs: {
+                    nodes: [
+                      {
+                        name: 'job_test_2',
+                        needs: { nodes: [], __typename: 'CiConfigNeedConnection' },
+                        __typename: 'CiConfigJob',
+                      },
+                    ],
+                    __typename: 'CiConfigJobConnection',
+                  },
+                  __typename: 'CiConfigGroup',
+                },
+              ],
+              __typename: 'CiConfigGroupConnection',
+            },
+            __typename: 'CiConfigStage',
+          },
+          {
+            name: 'build',
+            groups: {
+              nodes: [
+                {
+                  name: 'job_build',
+                  jobs: {
+                    nodes: [
+                      {
+                        name: 'job_build',
+                        needs: {
+                          nodes: [{ name: 'job_test_2', __typename: 'CiConfigNeed' }],
+                          __typename: 'CiConfigNeedConnection',
+                        },
+                        __typename: 'CiConfigJob',
+                      },
+                    ],
+                    __typename: 'CiConfigJobConnection',
+                  },
+                  __typename: 'CiConfigGroup',
+                },
+              ],
+              __typename: 'CiConfigGroupConnection',
+            },
+            __typename: 'CiConfigStage',
+          },
+        ],
+      },
+      __typename: 'CiConfig',
     },
   },
 };

spec/frontend/pipeline_editor/pipeline_editor_app_spec.js

@@ -27,7 +27,7 @@ import {
 } from './mock_data';
 
 import CommitForm from '~/pipeline_editor/components/commit/commit_form.vue';
-import getCiConfig from '~/pipeline_editor/graphql/queries/ci_config.graphql';
+import getCiConfigData from '~/pipeline_editor/graphql/queries/ci_config.graphql';
 import PipelineGraph from '~/pipelines/components/pipeline_graph/pipeline_graph.vue';
 import PipelineEditorApp from '~/pipeline_editor/pipeline_editor_app.vue';
 import TextEditor from '~/pipeline_editor/components/text_editor.vue';
@@ -112,7 +112,7 @@ describe('~/pipeline_editor/pipeline_editor_app.vue', () => {
   };
 
   const createComponentWithApollo = ({ props = {}, mountFn = shallowMount } = {}) => {
-    const handlers = [[getCiConfig, mockCiConfigData]];
+    const handlers = [[getCiConfigData, mockCiConfigData]];
     const resolvers = {
       Query: {
         blobContent() {
@@ -137,7 +137,6 @@ describe('~/pipeline_editor/pipeline_editor_app.vue', () => {
 
   const findLoadingIcon = () => wrapper.find(GlLoadingIcon);
   const findAlert = () => wrapper.find(GlAlert);
-  const findBlobFailureAlert = () => wrapper.find(GlAlert);
   const findTabAt = i => wrapper.findAll(GlTab).at(i);
   const findVisualizationTab = () => wrapper.find('[data-testid="visualization-tab"]');
   const findTextEditor = () => wrapper.find(TextEditor);
@@ -227,10 +226,12 @@ describe('~/pipeline_editor/pipeline_editor_app.vue', () => {
     beforeEach(async () => {
       createComponent({ mountFn: mount });
 
-      await wrapper.setData({
+      wrapper.setData({
         content: mockCiYml,
         contentModel: mockCiYml,
       });
+
+      await waitForPromises();
     });
 
     it('displays content after the query loads', () => {
@@ -352,7 +353,7 @@ describe('~/pipeline_editor/pipeline_editor_app.vue', () => {
       });
 
       describe('when the commit fails', () => {
-        it('shows a the error message', async () => {
+        it('shows an error message', async () => {
           mockMutate.mockRejectedValueOnce(new Error('commit failed'));
 
           await submitCommit();
@@ -401,7 +402,7 @@ describe('~/pipeline_editor/pipeline_editor_app.vue', () => {
 
       await waitForPromises();
 
-      expect(findBlobFailureAlert().exists()).toBe(false);
+      expect(findAlert().exists()).toBe(false);
       expect(findTextEditor().attributes('value')).toBe(mockCiYml);
     });
 
@@ -415,9 +416,7 @@ describe('~/pipeline_editor/pipeline_editor_app.vue', () => {
 
       await waitForPromises();
 
-      expect(findBlobFailureAlert().text()).toBe(
-        'No CI file found in this repository, please add one.',
-      );
+      expect(findAlert().text()).toBe('No CI file found in this repository, please add one.');
     });
 
     it('shows a 400 error message', async () => {
@@ -430,9 +429,7 @@ describe('~/pipeline_editor/pipeline_editor_app.vue', () => {
 
       await waitForPromises();
 
-      expect(findBlobFailureAlert().text()).toBe(
-        'Repository does not have a default branch, please set one.',
-      );
+      expect(findAlert().text()).toBe('Repository does not have a default branch, please set one.');
     });
 
     it('shows a unkown error message', async () => {
@@ -440,9 +437,7 @@ describe('~/pipeline_editor/pipeline_editor_app.vue', () => {
       createComponentWithApollo();
       await waitForPromises();
 
-      expect(findBlobFailureAlert().text()).toBe(
-        'The CI configuration was not loaded, please try again.',
-      );
+      expect(findAlert().text()).toBe('The CI configuration was not loaded, please try again.');
     });
   });
 });