From a5f4bba440d7f9ea47046a0a561d49adf0a1e6d4 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 16 Jun 2021 18:25:58 +0000 Subject: Add latest changes from gitlab-org/gitlab@14-0-stable-ee --- spec/tooling/danger/changelog_spec.rb | 299 +++++------ spec/tooling/danger/product_intelligence_spec.rb | 150 ++++++ spec/tooling/danger/project_helper_spec.rb | 36 +- spec/tooling/graphql/docs/renderer_spec.rb | 643 +++++++++++++++++++++++ 4 files changed, 951 insertions(+), 177 deletions(-) create mode 100644 spec/tooling/danger/product_intelligence_spec.rb create mode 100644 spec/tooling/graphql/docs/renderer_spec.rb (limited to 'spec/tooling') diff --git a/spec/tooling/danger/changelog_spec.rb b/spec/tooling/danger/changelog_spec.rb index 7637d894265..5777186cc28 100644 --- a/spec/tooling/danger/changelog_spec.rb +++ b/spec/tooling/danger/changelog_spec.rb @@ -18,136 +18,52 @@ RSpec.describe Tooling::Danger::Changelog do allow(changelog).to receive(:project_helper).and_return(fake_project_helper) end - describe '#check_changelog_trailer' do - subject { changelog.check_changelog_trailer(commit) } - - context "when commit doesn't include a changelog trailer" do - let(:commit) { double('commit', message: "Hello world") } - - it { is_expected.to be_nil } - end - - context "when commit include a changelog trailer with no category" do - let(:commit) { double('commit', message: "Hello world\n\nChangelog:") } + describe '#check_changelog_commit_categories' do + context 'when all changelog commits are correct' do + it 'does not produce any messages' do + commit = double(:commit, message: "foo\nChangelog: fixed") - it { is_expected.to be_nil } - end + allow(changelog).to receive(:changelog_commits).and_return([commit]) - context "when commit include a changelog trailer with an unknown category" do - let(:commit) { double('commit', message: "Hello world\n\nChangelog: foo", sha: "abc123") } - - it { is_expected.to have_attributes(errors: ["Commit #{commit.sha} uses an invalid changelog category: foo"]) } - end - - described_class::CATEGORIES.each do |category| - context "when commit include a changelog trailer with category set to '#{category}'" do - let(:commit) { double('commit', message: "Hello world\n\nChangelog: #{category}", sha: "abc123") } + expect(changelog).not_to receive(:fail) - it { is_expected.to have_attributes(errors: []) } + changelog.check_changelog_commit_categories end end - end - - describe '#check_changelog_yaml' do - let(:changelog_path) { 'ee/changelogs/unreleased/entry.yml' } - let(:changes) { changes_class.new([change_class.new(changelog_path, :added, :changelog)]) } - let(:yaml_title) { 'Fix changelog Dangerfile to convert MR IID to a string before comparison' } - let(:yaml_merge_request) { 60899 } - let(:mr_iid) { '60899' } - let(:yaml_type) { 'fixed' } - let(:yaml) do - <<~YAML - --- - title: #{yaml_title} - merge_request: #{yaml_merge_request} - author: - type: #{yaml_type} - YAML - end - - before do - allow(changelog).to receive(:present?).and_return(true) - allow(changelog).to receive(:changelog_path).and_return(changelog_path) - allow(changelog).to receive(:read_file).with(changelog_path).and_return(yaml) - allow(fake_helper).to receive(:security_mr?).and_return(false) - allow(fake_helper).to receive(:mr_iid).and_return(mr_iid) - allow(fake_helper).to receive(:cherry_pick_mr?).and_return(false) - allow(fake_helper).to receive(:stable_branch?).and_return(false) - allow(fake_helper).to receive(:html_link).with(changelog_path).and_return(changelog_path) - end - - subject { changelog.check_changelog_yaml } - - context "when changelog is not present" do - before do - allow(changelog).to receive(:present?).and_return(false) - end - it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } - end + context 'when a commit has an incorrect trailer' do + it 'adds a message' do + commit = double(:commit, message: "foo\nChangelog: foo", sha: '123') - context "when YAML is invalid" do - let(:yaml) { '{ foo bar]' } + allow(changelog).to receive(:changelog_commits).and_return([commit]) - it { is_expected.to have_attributes(errors: ["#{changelog_path} isn't valid YAML! #{described_class::SEE_DOC}"]) } - end + expect(changelog).to receive(:fail) - context "when a StandardError is raised" do - before do - allow(changelog).to receive(:read_file).and_raise(StandardError, "Fail!") + changelog.check_changelog_commit_categories end - - it { is_expected.to have_attributes(warnings: ["There was a problem trying to check the Changelog. Exception: StandardError - Fail!"]) } - end - - context "when YAML title is nil" do - let(:yaml_title) { '' } - - it { is_expected.to have_attributes(errors: ["`title` should be set, in #{changelog_path}! #{described_class::SEE_DOC}"]) } - end - - context "when YAML type is nil" do - let(:yaml_type) { '' } - - it { is_expected.to have_attributes(errors: ["`type` should be set, in #{changelog_path}! #{described_class::SEE_DOC}"]) } end + end - context "when on a security MR" do - let(:yaml_merge_request) { '' } + describe '#check_changelog_trailer' do + subject { changelog.check_changelog_trailer(commit) } - before do - allow(fake_helper).to receive(:security_mr?).and_return(true) - end + context "when commit include a changelog trailer with an unknown category" do + let(:commit) { double('commit', message: "Hello world\n\nChangelog: foo", sha: "abc123") } - it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + it { is_expected.to have_attributes(errors: ["Commit #{commit.sha} uses an invalid changelog category: foo"]) } end - context "when MR IID is empty" do - before do - allow(fake_helper).to receive(:mr_iid).and_return("") - end + context 'when a commit uses the wrong casing for a trailer' do + let(:commit) { double('commit', message: "Hello world\n\nchangelog: foo", sha: "abc123") } - it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + it { is_expected.to have_attributes(errors: ["The changelog trailer for commit #{commit.sha} must be `Changelog` (starting with a capital C), not `changelog`"]) } end - context "when YAML MR IID is empty" do - let(:yaml_merge_request) { '' } - - context "and YAML includes a merge_request: line" do - it { is_expected.to have_attributes(markdowns: [{ msg: format(described_class::SUGGEST_MR_COMMENT, mr_iid: fake_helper.mr_iid), file: changelog_path, line: 3 }]) } - end - - context "and YAML does not include a merge_request: line" do - let(:yaml) do - <<~YAML - --- - title: #{yaml_title} - author: - type: #{yaml_type} - YAML - end + described_class::CATEGORIES.each do |category| + context "when commit include a changelog trailer with category set to '#{category}'" do + let(:commit) { double('commit', message: "Hello world\n\nChangelog: #{category}", sha: "abc123") } - it { is_expected.to have_attributes(messages: ["Consider setting `merge_request` to #{mr_iid} in #{changelog_path}. #{described_class::SEE_DOC}"]) } + it { is_expected.to have_attributes(errors: []) } end end end @@ -177,13 +93,26 @@ RSpec.describe Tooling::Danger::Changelog do let(:ee_change) { change_class.new('ee/app/models/foo.rb', :added, :backend) } context "and a non-EE changelog, and changelog not required" do - let(:changelog_change) { change_class.new('changelogs/unreleased/entry.yml', :added, :changelog) } - before do allow(changelog).to receive(:required?).and_return(false) + allow(changelog).to receive(:ee_changelog?).and_return(false) end - it { is_expected.to have_attributes(warnings: ["This MR has a Changelog file outside `ee/`, but code changes in `ee/`. Consider moving the Changelog file into `ee/`."]) } + it { is_expected.to have_attributes(warnings: ["This MR changes code in `ee/`, but its Changelog commit is missing the [`EE: true` trailer](https://docs.gitlab.com/ee/development/changelog.html#gitlab-enterprise-changes). Consider adding it to your Changelog commits."]) } + end + + context "and a EE changelog" do + before do + allow(changelog).to receive(:ee_changelog?).and_return(true) + end + + it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } + + context "and there are DB changes" do + let(:foss_change) { change_class.new('db/migrate/foo.rb', :added, :migration) } + + it { is_expected.to have_attributes(warnings: ["This MR has a Changelog commit with the `EE: true` trailer, but there are database changes which [requires](https://docs.gitlab.com/ee/development/changelog.html#what-warrants-a-changelog-entry) the Changelog commit to not have the `EE: true` trailer. Consider removing the `EE: true` trailer from your commits."]) } + end end end @@ -191,15 +120,19 @@ RSpec.describe Tooling::Danger::Changelog do let(:foss_change) { change_class.new('app/models/foo.rb', :added, :backend) } context "and a non-EE changelog" do - let(:changelog_change) { change_class.new('changelogs/unreleased/entry.yml', :added, :changelog) } + before do + allow(changelog).to receive(:ee_changelog?).and_return(false) + end it { is_expected.to have_attributes(errors: [], warnings: [], markdowns: [], messages: []) } end context "and a EE changelog" do - let(:changelog_change) { change_class.new('ee/changelogs/unreleased/entry.yml', :added, :changelog) } + before do + allow(changelog).to receive(:ee_changelog?).and_return(true) + end - it { is_expected.to have_attributes(warnings: ["This MR has a Changelog file in `ee/`, but no code changes in `ee/`. Consider moving the Changelog file outside `ee/`."]) } + it { is_expected.to have_attributes(warnings: ["This MR has a Changelog commit for EE, but no code changes in `ee/`. Consider removing the `EE: true` trailer from your commits."]) } end end end @@ -207,20 +140,26 @@ RSpec.describe Tooling::Danger::Changelog do describe '#required_reasons' do subject { changelog.required_reasons } + context "added files contain a migration" do + let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } + + it { is_expected.to include(:db_changes) } + end + context "removed files contains a feature flag" do let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) } it { is_expected.to include(:feature_flag_removed) } end - context "removed files do not contain a feature flag" do - let(:changes) { changes_class.new([change_class.new('foo', :deleted, :backend)]) } + context "added files do not contain a migration" do + let(:changes) { changes_class.new([change_class.new('foo', :added, :frontend)]) } it { is_expected.to be_empty } end - context "added files contain a migration" do - let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } + context "removed files do not contain a feature flag" do + let(:changes) { changes_class.new([change_class.new('foo', :deleted, :backend)]) } it { is_expected.to be_empty } end @@ -229,20 +168,26 @@ RSpec.describe Tooling::Danger::Changelog do describe '#required?' do subject { changelog.required? } + context 'added files contain a migration' do + let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } + + it { is_expected.to be_truthy } + end + context "removed files contains a feature flag" do let(:changes) { changes_class.new([change_class.new('foo', :deleted, :feature_flag)]) } it { is_expected.to be_truthy } end - context "removed files do not contain a feature flag" do - let(:changes) { changes_class.new([change_class.new('foo', :deleted, :backend)]) } + context 'added files do not contain a migration' do + let(:changes) { changes_class.new([change_class.new('foo', :added, :frontend)]) } it { is_expected.to be_falsey } end - context "added files contain a migration" do - let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } + context "removed files do not contain a feature flag" do + let(:changes) { changes_class.new([change_class.new('foo', :deleted, :backend)]) } it { is_expected.to be_falsey } end @@ -301,50 +246,63 @@ RSpec.describe Tooling::Danger::Changelog do end describe '#present?' do - subject { changelog.present? } + it 'returns true when a Changelog commit is present' do + allow(changelog) + .to receive(:valid_changelog_commits) + .and_return([double(:commit)]) - context 'added files contain a changelog' do - let(:changes) { changes_class.new([change_class.new('foo', :added, :changelog)]) } - - it { is_expected.to be_truthy } + expect(changelog).to be_present end - context 'added files do not contain a changelog' do - let(:changes) { changes_class.new([change_class.new('foo', :added, :backend)]) } + it 'returns false when a Changelog commit is missing' do + allow(changelog).to receive(:valid_changelog_commits).and_return([]) - it { is_expected.to be_falsy } + expect(changelog).not_to be_present end end - describe '#ee_changelog?' do - subject { changelog.ee_changelog? } + describe '#changelog_commits' do + it 'returns the commits that include a Changelog trailer' do + commit1 = double(:commit, message: "foo\nChangelog: fixed") + commit2 = double(:commit, message: "bar\nChangelog: kittens") + commit3 = double(:commit, message: 'testing') + git = double(:git) - context 'is ee changelog' do - let(:changes) { changes_class.new([change_class.new('ee/changelogs/unreleased/entry.yml', :added, :changelog)]) } + allow(changelog).to receive(:git).and_return(git) + allow(git).to receive(:commits).and_return([commit1, commit2, commit3]) - it { is_expected.to be_truthy } + expect(changelog.changelog_commits).to eq([commit1, commit2]) end + end + + describe '#valid_changelog_commits' do + it 'returns the commits with a valid Changelog trailer' do + commit1 = double(:commit, message: "foo\nChangelog: fixed") + commit2 = double(:commit, message: "bar\nChangelog: kittens") - context 'is not ee changelog' do - let(:changes) { changes_class.new([change_class.new('changelogs/unreleased/entry.yml', :added, :changelog)]) } + allow(changelog) + .to receive(:changelog_commits) + .and_return([commit1, commit2]) - it { is_expected.to be_falsy } + expect(changelog.valid_changelog_commits).to eq([commit1]) end end - describe '#changelog_path' do - subject { changelog.changelog_path } + describe '#ee_changelog?' do + it 'returns true when an EE changelog commit is present' do + commit = double(:commit, message: "foo\nEE: true") - context 'added files contain a changelog' do - let(:changes) { changes_class.new([change_class.new('foo', :added, :changelog)]) } + allow(changelog).to receive(:changelog_commits).and_return([commit]) - it { is_expected.to eq('foo') } + expect(changelog.ee_changelog?).to eq(true) end - context 'added files do not contain a changelog' do - let(:changes) { changes_class.new([change_class.new('foo', :added, :backend)]) } + it 'returns false when an EE changelog commit is missing' do + commit = double(:commit, message: 'foo') - it { is_expected.to be_nil } + allow(changelog).to receive(:changelog_commits).and_return([commit]) + + expect(changelog.ee_changelog?).to eq(false) end end @@ -355,8 +313,8 @@ RSpec.describe Tooling::Danger::Changelog do shared_examples 'changelog modified text' do |key| specify do expect(subject).to include('CHANGELOG.md was edited') - expect(subject).to include('bin/changelog -m 1234 "Fake Title"') - expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"') + expect(subject).to include('`Changelog` trailer') + expect(subject).to include('`EE: true`') end end @@ -386,7 +344,7 @@ RSpec.describe Tooling::Danger::Changelog do specify do expect(subject).to include('CHANGELOG.md was edited') - expect(subject).not_to include('bin/changelog') + expect(subject).not_to include('`Changelog` trailer') end end end @@ -405,8 +363,21 @@ RSpec.describe Tooling::Danger::Changelog do specify do expect(subject).to have_key(key) expect(subject[key]).to include('CHANGELOG missing') - expect(subject[key]).to include('bin/changelog -m 1234 "Fake Title"') - expect(subject[key]).not_to include('--ee') + expect(subject[key]).to include('`Changelog` trailer') + end + end + + context 'with a new migration file' do + let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } + + context "when title is not changed from sanitization", :aggregate_failures do + it_behaves_like 'changelog required text', :db_changes + end + + context "when title needs sanitization", :aggregate_failures do + let(:mr_title) { 'DRAFT: Fake Title' } + + it_behaves_like 'changelog required text', :db_changes end end @@ -426,8 +397,21 @@ RSpec.describe Tooling::Danger::Changelog do specify do expect(subject).to have_key(key) expect(subject[key]).to include('CHANGELOG missing') - expect(subject[key]).not_to include('bin/changelog') - expect(subject[key]).not_to include('--ee') + expect(subject[key]).not_to include('`Changelog` trailer') + end + end + + context 'with a new migration file' do + let(:changes) { changes_class.new([change_class.new('foo', :added, :migration)]) } + + context "when title is not changed from sanitization", :aggregate_failures do + it_behaves_like 'changelog required text', :db_changes + end + + context "when title needs sanitization", :aggregate_failures do + let(:mr_title) { 'DRAFT: Fake Title' } + + it_behaves_like 'changelog required text', :db_changes end end @@ -446,8 +430,8 @@ RSpec.describe Tooling::Danger::Changelog do shared_examples 'changelog optional text' do |key| specify do expect(subject).to include('CHANGELOG missing') - expect(subject).to include('bin/changelog -m 1234 "Fake Title"') - expect(subject).to include('bin/changelog --ee -m 1234 "Fake Title"') + expect(subject).to include('`Changelog` trailer') + expect(subject).to include('EE: true') end end @@ -477,7 +461,6 @@ RSpec.describe Tooling::Danger::Changelog do specify do expect(subject).to include('CHANGELOG missing') - expect(subject).not_to include('bin/changelog') end end end diff --git a/spec/tooling/danger/product_intelligence_spec.rb b/spec/tooling/danger/product_intelligence_spec.rb new file mode 100644 index 00000000000..17ef67e64fe --- /dev/null +++ b/spec/tooling/danger/product_intelligence_spec.rb @@ -0,0 +1,150 @@ +# frozen_string_literal: true + +require 'gitlab-dangerfiles' +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../tooling/danger/product_intelligence' +require_relative '../../../tooling/danger/project_helper' + +RSpec.describe Tooling::Danger::ProductIntelligence do + include_context "with dangerfile" + + subject(:product_intelligence) { fake_danger.new(helper: fake_helper) } + + let(:fake_danger) { DangerSpecHelper.fake_danger.include(described_class) } + let(:changed_files) { ['metrics/counts_7d/test_metric.yml', 'doc/development/usage_ping/dictionary.md'] } + let(:changed_lines) { ['+tier: ee'] } + + before do + allow(fake_helper).to receive(:all_changed_files).and_return(changed_files) + allow(fake_helper).to receive(:changed_lines).and_return(changed_lines) + end + + describe '#need_dictionary_changes?' do + subject { product_intelligence.need_dictionary_changes? } + + context 'when changed files do not contain dictionary changes' do + let(:changed_files) { ['config/metrics/counts_7d/test_metric.yml'] } + + it { is_expected.to be true } + end + + context 'when changed files already contains dictionary changes' do + let(:changed_files) { ['doc/development/usage_ping/dictionary.md'] } + + it { is_expected.to be false } + end + end + + describe '#missing_labels' do + subject { product_intelligence.missing_labels } + + let(:ci_env) { true } + + before do + allow(fake_helper).to receive(:mr_has_labels?).and_return(false) + allow(fake_helper).to receive(:ci?).and_return(ci_env) + end + + context 'with ci? false' do + let(:ci_env) { false } + + it { is_expected.to be_empty } + end + + context 'with ci? true' do + let(:expected_labels) { ['product intelligence', 'product intelligence::review pending'] } + + it { is_expected.to match_array(expected_labels) } + end + + context 'with product intelligence label' do + let(:expected_labels) { ['product intelligence::review pending'] } + + before do + allow(fake_helper).to receive(:mr_has_labels?).with('product intelligence').and_return(true) + end + + it { is_expected.to match_array(expected_labels) } + end + + context 'with product intelligence::review pending' do + before do + allow(fake_helper).to receive(:mr_has_labels?).and_return(true) + end + + it { is_expected.to be_empty } + end + end + + describe '#matching_changed_files' do + subject { product_intelligence.matching_changed_files } + + let(:changed_files) do + [ + 'dashboard/todos_controller.rb', + 'components/welcome.vue', + 'admin/groups/_form.html.haml' + ] + end + + context 'with snowplow files changed' do + context 'when vue file changed' do + let(:changed_lines) { ['+data-track-event'] } + + it { is_expected.to match_array(['components/welcome.vue']) } + end + + context 'when haml file changed' do + let(:changed_lines) { ['+ data: { track_label:'] } + + it { is_expected.to match_array(['admin/groups/_form.html.haml']) } + end + + context 'when ruby file changed' do + let(:changed_lines) { ['+ Gitlab::Tracking.event'] } + let(:changed_files) { ['dashboard/todos_controller.rb', 'admin/groups/_form.html.haml'] } + + it { is_expected.to match_array(['dashboard/todos_controller.rb']) } + end + end + + context 'with dictionary file not changed' do + it { is_expected.to be_empty } + end + + context 'with metrics files changed' do + let(:changed_files) { ['config/metrics/counts_7d/test_metric.yml', 'ee/config/metrics/counts_7d/ee_metric.yml'] } + + it { is_expected.to match_array(changed_files) } + end + + context 'with metrics files not changed' do + it { is_expected.to be_empty } + end + + context 'with tracking files changed' do + let(:changed_files) do + [ + 'lib/gitlab/tracking.rb', + 'spec/lib/gitlab/tracking_spec.rb', + 'app/helpers/tracking_helper.rb' + ] + end + + it { is_expected.to match_array(changed_files) } + end + + context 'with usage_data files changed' do + let(:changed_files) do + [ + 'doc/api/usage_data.md', + 'ee/lib/ee/gitlab/usage_data.rb', + 'spec/lib/gitlab/usage_data_spec.rb' + ] + end + + it { is_expected.to match_array(changed_files) } + end + end +end diff --git a/spec/tooling/danger/project_helper_spec.rb b/spec/tooling/danger/project_helper_spec.rb index 1d2ea0f5ba3..7474709d255 100644 --- a/spec/tooling/danger/project_helper_spec.rb +++ b/spec/tooling/danger/project_helper_spec.rb @@ -112,10 +112,10 @@ RSpec.describe Tooling::Danger::ProjectHelper do 'FOO_VERSION' | [:backend] 'Dangerfile' | [:engineering_productivity] - 'danger/commit_messages/Dangerfile' | [:engineering_productivity] - 'ee/danger/commit_messages/Dangerfile' | [:engineering_productivity] - 'danger/commit_messages/' | [:engineering_productivity] - 'ee/danger/commit_messages/' | [:engineering_productivity] + 'danger/bundle_size/Dangerfile' | [:engineering_productivity] + 'ee/danger/bundle_size/Dangerfile' | [:engineering_productivity] + 'danger/bundle_size/' | [:engineering_productivity] + 'ee/danger/bundle_size/' | [:engineering_productivity] '.gitlab-ci.yml' | [:engineering_productivity] '.gitlab/ci/cng.gitlab-ci.yml' | [:engineering_productivity] '.gitlab/ci/ee-specific-checks.gitlab-ci.yml' | [:engineering_productivity] @@ -139,18 +139,18 @@ RSpec.describe Tooling::Danger::ProjectHelper do 'db/post_migrate/foo' | [:database, :migration] 'ee/db/geo/migrate/foo' | [:database, :migration] 'ee/db/geo/post_migrate/foo' | [:database, :migration] - 'app/models/project_authorization.rb' | [:database] - 'app/services/users/refresh_authorized_projects_service.rb' | [:database] - 'app/services/authorized_project_update/find_records_due_for_refresh_service.rb' | [:database] - 'lib/gitlab/background_migration.rb' | [:database] - 'lib/gitlab/background_migration/foo' | [:database] - 'ee/lib/gitlab/background_migration/foo' | [:database] - 'lib/gitlab/database.rb' | [:database] - 'lib/gitlab/database/foo' | [:database] - 'ee/lib/gitlab/database/foo' | [:database] - 'lib/gitlab/github_import.rb' | [:database] - 'lib/gitlab/github_import/foo' | [:database] - 'lib/gitlab/sql/foo' | [:database] + 'app/models/project_authorization.rb' | [:database, :backend] + 'app/services/users/refresh_authorized_projects_service.rb' | [:database, :backend] + 'app/services/authorized_project_update/find_records_due_for_refresh_service.rb' | [:database, :backend] + 'lib/gitlab/background_migration.rb' | [:database, :backend] + 'lib/gitlab/background_migration/foo' | [:database, :backend] + 'ee/lib/gitlab/background_migration/foo' | [:database, :backend] + 'lib/gitlab/database.rb' | [:database, :backend] + 'lib/gitlab/database/foo' | [:database, :backend] + 'ee/lib/gitlab/database/foo' | [:database, :backend] + 'lib/gitlab/github_import.rb' | [:database, :backend] + 'lib/gitlab/github_import/foo' | [:database, :backend] + 'lib/gitlab/sql/foo' | [:database, :backend] 'rubocop/cop/migration/foo' | [:database] 'db/fixtures/foo.rb' | [:backend] @@ -162,8 +162,6 @@ RSpec.describe Tooling::Danger::ProjectHelper do 'workhorse/main.go' | [:workhorse] 'workhorse/internal/upload/upload.go' | [:workhorse] - 'changelogs/foo' | [:none] - 'ee/changelogs/foo' | [:none] 'locale/gitlab.pot' | [:none] 'FOO' | [:unknown] @@ -220,7 +218,7 @@ RSpec.describe Tooling::Danger::ProjectHelper do describe '.local_warning_message' do it 'returns an informational message with rules that can run' do - expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: changelog, commit_messages, database, datateam, documentation, duplicate_yarn_dependencies, eslint, karma, pajamas, pipeline, prettier, product_intelligence, utility_css') + expect(described_class.local_warning_message).to eq('==> Only the following Danger rules can be run locally: changelog, database, datateam, documentation, duplicate_yarn_dependencies, eslint, karma, pajamas, pipeline, prettier, product_intelligence, utility_css') end end diff --git a/spec/tooling/graphql/docs/renderer_spec.rb b/spec/tooling/graphql/docs/renderer_spec.rb new file mode 100644 index 00000000000..50ebb754ca4 --- /dev/null +++ b/spec/tooling/graphql/docs/renderer_spec.rb @@ -0,0 +1,643 @@ +# frozen_string_literal: true + +require_relative '../../../../tooling/graphql/docs/renderer' + +RSpec.describe Tooling::Graphql::Docs::Renderer do + describe '#contents' do + shared_examples 'renders correctly as GraphQL documentation' do + it 'contains the expected section' do + # duplicative - but much better error messages! + section.lines.each { |line| expect(contents).to include(line) } + expect(contents).to include(section) + end + end + + let(:template) { Rails.root.join('tooling/graphql/docs/templates/default.md.haml') } + let(:field_description) { 'List of objects.' } + let(:type) { ::GraphQL::INT_TYPE } + + let(:query_type) do + Class.new(Types::BaseObject) { graphql_name 'Query' }.tap do |t| + # this keeps type and field_description in scope. + t.field :foo, type, null: true, description: field_description do + argument :id, GraphQL::ID_TYPE, required: false, description: 'ID of the object.' + end + end + end + + let(:mutation_root) do + Class.new(::Types::BaseObject) do + include ::Gitlab::Graphql::MountMutation + graphql_name 'Mutation' + end + end + + let(:mock_schema) do + Class.new(GraphQL::Schema) do + def resolve_type(obj, ctx) + raise 'Not a real schema' + end + end + end + + subject(:contents) do + mock_schema.query(query_type) + mock_schema.mutation(mutation_root) if mutation_root.fields.any? + + described_class.new( + mock_schema, + output_dir: nil, + template: template + ).contents + end + + describe 'headings' do + it 'contains the expected sections' do + expect(contents.lines.map(&:chomp)).to include( + '## `Query` type', + '## `Mutation` type', + '## Connections', + '## Object types', + '## Enumeration types', + '## Scalar types', + '## Abstract types', + '### Unions', + '### Interfaces', + '## Input types' + ) + end + end + + context 'when a field has a list type' do + let(:type) do + Class.new(Types::BaseObject) do + graphql_name 'ArrayTest' + + field :foo, [GraphQL::STRING_TYPE], null: false, description: 'A description.' + end + end + + specify do + type_name = '[String!]!' + inner_type = 'string' + expectation = <<~DOC + ### `ArrayTest` + + #### Fields + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `foo` | [`#{type_name}`](##{inner_type}) | A description. | + DOC + + is_expected.to include(expectation) + end + + describe 'a top level query field' do + let(:expectation) do + <<~DOC + ### `Query.foo` + + List of objects. + + Returns [`ArrayTest`](#arraytest). + + #### Arguments + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `id` | [`ID`](#id) | ID of the object. | + DOC + end + + it 'generates the query with arguments' do + expect(subject).to include(expectation) + end + + context 'when description does not end with `.`' do + let(:field_description) { 'List of objects' } + + it 'adds the `.` to the end' do + expect(subject).to include(expectation) + end + end + end + end + + describe 'when fields are not defined in alphabetical order' do + let(:type) do + Class.new(Types::BaseObject) do + graphql_name 'OrderingTest' + + field :foo, GraphQL::STRING_TYPE, null: false, description: 'A description of foo field.' + field :bar, GraphQL::STRING_TYPE, null: false, description: 'A description of bar field.' + end + end + + it 'lists the fields in alphabetical order' do + expectation = <<~DOC + ### `OrderingTest` + + #### Fields + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `bar` | [`String!`](#string) | A description of bar field. | + | `foo` | [`String!`](#string) | A description of foo field. | + DOC + + is_expected.to include(expectation) + end + end + + context 'when a field has a documentation reference' do + let(:type) do + wibble = Class.new(::Types::BaseObject) do + graphql_name 'Wibble' + field :x, ::GraphQL::INT_TYPE, null: false + end + + Class.new(Types::BaseObject) do + graphql_name 'DocRefSpec' + description 'Testing doc refs' + + field :foo, + type: GraphQL::STRING_TYPE, + null: false, + description: 'The foo.', + see: { 'A list of foos' => 'https://example.com/foos' } + field :bar, + type: GraphQL::STRING_TYPE, + null: false, + description: 'The bar.', + see: { 'A list of bars' => 'https://example.com/bars' } do + argument :barity, ::GraphQL::INT_TYPE, required: false, description: '?' + end + field :wibbles, + type: wibble.connection_type, + null: true, + description: 'The wibbles', + see: { 'wibblance' => 'https://example.com/wibbles' } + end + end + + let(:section) do + <<~DOC + ### `DocRefSpec` + + Testing doc refs. + + #### Fields + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `foo` | [`String!`](#string) | The foo. See [A list of foos](https://example.com/foos). | + | `wibbles` | [`WibbleConnection`](#wibbleconnection) | The wibbles. See [wibblance](https://example.com/wibbles). (see [Connections](#connections)) | + + #### Fields with arguments + + ##### `DocRefSpec.bar` + + The bar. See [A list of bars](https://example.com/bars). + + Returns [`String!`](#string). + + ###### Arguments + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `barity` | [`Int`](#int) | ?. | + DOC + end + + it_behaves_like 'renders correctly as GraphQL documentation' + end + + context 'when an argument is deprecated' do + let(:type) do + Class.new(Types::BaseObject) do + graphql_name 'DeprecatedTest' + description 'A thing we used to use, but no longer support' + + field :foo, + type: GraphQL::STRING_TYPE, + null: false, + description: 'A description.' do + argument :foo_arg, GraphQL::STRING_TYPE, + required: false, + description: 'The argument.', + deprecated: { reason: 'Bad argument', milestone: '101.2' } + end + end + end + + let(:section) do + <<~DOC + ##### `DeprecatedTest.foo` + + A description. + + Returns [`String!`](#string). + + ###### Arguments + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `fooArg` **{warning-solid}** | [`String`](#string) | **Deprecated** in 101.2. Bad argument. | + DOC + end + + it_behaves_like 'renders correctly as GraphQL documentation' + end + + context 'when a field is deprecated' do + let(:type) do + Class.new(Types::BaseObject) do + graphql_name 'DeprecatedTest' + description 'A thing we used to use, but no longer support' + + field :foo, + type: GraphQL::STRING_TYPE, + null: false, + deprecated: { reason: 'This is deprecated', milestone: '1.10' }, + description: 'A description.' + field :foo_with_args, + type: GraphQL::STRING_TYPE, + null: false, + deprecated: { reason: 'Do not use', milestone: '1.10', replacement: 'X.y' }, + description: 'A description.' do + argument :arg, GraphQL::INT_TYPE, required: false, description: 'Argity' + end + field :bar, + type: GraphQL::STRING_TYPE, + null: false, + description: 'A description.', + deprecated: { + reason: :renamed, + milestone: '1.10', + replacement: 'Query.boom' + } + end + end + + let(:section) do + <<~DOC + ### `DeprecatedTest` + + A thing we used to use, but no longer support. + + #### Fields + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `bar` **{warning-solid}** | [`String!`](#string) | **Deprecated** in 1.10. This was renamed. Use: [`Query.boom`](#queryboom). | + | `foo` **{warning-solid}** | [`String!`](#string) | **Deprecated** in 1.10. This is deprecated. | + + #### Fields with arguments + + ##### `DeprecatedTest.fooWithArgs` + + A description. + + WARNING: + **Deprecated** in 1.10. + Do not use. + Use: [`X.y`](#xy). + + Returns [`String!`](#string). + + ###### Arguments + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `arg` | [`Int`](#int) | Argity. | + DOC + end + + it_behaves_like 'renders correctly as GraphQL documentation' + end + + context 'when a Query.field is deprecated' do + before do + query_type.field( + name: :bar, + type: type, + null: true, + description: 'A bar', + deprecated: { reason: :renamed, milestone: '10.11', replacement: 'Query.foo' } + ) + end + + let(:type) { ::GraphQL::INT_TYPE } + let(:section) do + <<~DOC + ### `Query.bar` + + A bar. + + WARNING: + **Deprecated** in 10.11. + This was renamed. + Use: [`Query.foo`](#queryfoo). + + Returns [`Int`](#int). + DOC + end + + it_behaves_like 'renders correctly as GraphQL documentation' + end + + context 'when a field has an Enumeration type' do + let(:type) do + enum_type = Class.new(Types::BaseEnum) do + graphql_name 'MyEnum' + description 'A test of an enum.' + + value 'BAZ', + description: 'A description of BAZ.' + value 'BAR', + description: 'A description of BAR.', + deprecated: { reason: 'This is deprecated', milestone: '1.10' } + value 'BOOP', + description: 'A description of BOOP.', + deprecated: { reason: :renamed, replacement: 'MyEnum.BAR', milestone: '1.10' } + end + + Class.new(Types::BaseObject) do + graphql_name 'EnumTest' + + field :foo, enum_type, null: false, description: 'A description of foo field.' + end + end + + let(:section) do + <<~DOC + ### `MyEnum` + + A test of an enum. + + | Value | Description | + | ----- | ----------- | + | `BAR` **{warning-solid}** | **Deprecated** in 1.10. This is deprecated. | + | `BAZ` | A description of BAZ. | + | `BOOP` **{warning-solid}** | **Deprecated** in 1.10. This was renamed. Use: [`MyEnum.BAR`](#myenumbar). | + DOC + end + + it_behaves_like 'renders correctly as GraphQL documentation' + end + + context 'when a field has a global ID type' do + let(:type) do + Class.new(Types::BaseObject) do + graphql_name 'IDTest' + description 'A test for rendering IDs.' + + field :foo, ::Types::GlobalIDType[::User], null: true, description: 'A user foo.' + end + end + + describe 'section for IDTest' do + let(:section) do + <<~DOC + ### `IDTest` + + A test for rendering IDs. + + #### Fields + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `foo` | [`UserID`](#userid) | A user foo. | + DOC + end + + it_behaves_like 'renders correctly as GraphQL documentation' + end + + describe 'section for UserID' do + let(:section) do + <<~DOC + ### `UserID` + + A `UserID` is a global ID. It is encoded as a string. + + An example `UserID` is: `"gid://gitlab/User/1"`. + DOC + end + + it_behaves_like 'renders correctly as GraphQL documentation' + end + end + + context 'when there is a mutation' do + let(:mutation) do + mutation = Class.new(::Mutations::BaseMutation) + + mutation.graphql_name 'MakeItPretty' + mutation.description 'Make everything very pretty.' + + mutation.argument :prettiness_factor, + type: GraphQL::FLOAT_TYPE, + required: true, + description: 'How much prettier?' + + mutation.argument :pulchritude, + type: GraphQL::FLOAT_TYPE, + required: false, + description: 'How much prettier?', + deprecated: { + reason: :renamed, + replacement: 'prettinessFactor', + milestone: '72.34' + } + + mutation.field :everything, + type: GraphQL::STRING_TYPE, + null: true, + description: 'What we made prettier.' + + mutation.field :omnis, + type: GraphQL::STRING_TYPE, + null: true, + description: 'What we made prettier.', + deprecated: { + reason: :renamed, + replacement: 'everything', + milestone: '72.34' + } + + mutation + end + + before do + mutation_root.mount_mutation mutation + end + + it_behaves_like 'renders correctly as GraphQL documentation' do + let(:section) do + <<~DOC + ### `Mutation.makeItPretty` + + Make everything very pretty. + + Input type: `MakeItPrettyInput` + + #### Arguments + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | + | `prettinessFactor` | [`Float!`](#float) | How much prettier?. | + | `pulchritude` **{warning-solid}** | [`Float`](#float) | **Deprecated:** This was renamed. Please use `prettinessFactor`. Deprecated in 72.34. | + + #### Fields + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | + | `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | + | `everything` | [`String`](#string) | What we made prettier. | + | `omnis` **{warning-solid}** | [`String`](#string) | **Deprecated:** This was renamed. Please use `everything`. Deprecated in 72.34. | + DOC + end + end + + it 'does not render the automatically generated payload type' do + expect(contents).not_to include('MakeItPrettyPayload') + end + + it 'does not render the automatically generated input type as its own section' do + expect(contents).not_to include('# `MakeItPrettyInput`') + end + end + + context 'when there is an input type' do + let(:type) do + Class.new(::Types::BaseObject) do + graphql_name 'Foo' + field :wibble, type: ::GraphQL::INT_TYPE, null: true do + argument :date_range, + type: ::Types::TimeframeInputType, + required: true, + description: 'When the foo happened.' + end + end + end + + let(:section) do + <<~DOC + ### `Timeframe` + + A time-frame defined as a closed inclusive range of two dates. + + #### Arguments + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `end` | [`Date!`](#date) | The end of the range. | + | `start` | [`Date!`](#date) | The start of the range. | + DOC + end + + it_behaves_like 'renders correctly as GraphQL documentation' + end + + context 'when there is an interface and a union' do + let(:type) do + user = Class.new(::Types::BaseObject) + user.graphql_name 'User' + user.field :user_field, ::GraphQL::STRING_TYPE, null: true + group = Class.new(::Types::BaseObject) + group.graphql_name 'Group' + group.field :group_field, ::GraphQL::STRING_TYPE, null: true + + union = Class.new(::Types::BaseUnion) + union.graphql_name 'UserOrGroup' + union.description 'Either a user or a group.' + union.possible_types user, group + + interface = Module.new + interface.include(::Types::BaseInterface) + interface.graphql_name 'Flying' + interface.description 'Something that can fly.' + interface.field :flight_speed, GraphQL::INT_TYPE, null: true, description: 'Speed in mph.' + + african_swallow = Class.new(::Types::BaseObject) + african_swallow.graphql_name 'AfricanSwallow' + african_swallow.description 'A swallow from Africa.' + african_swallow.implements interface + interface.orphan_types african_swallow + + Class.new(::Types::BaseObject) do + graphql_name 'AbstractTypeTest' + description 'A test for abstract types.' + + field :foo, union, null: true, description: 'The foo.' + field :flying, interface, null: true, description: 'A flying thing.' + end + end + + it 'lists the fields correctly, and includes descriptions of all the types' do + type_section = <<~DOC + ### `AbstractTypeTest` + + A test for abstract types. + + #### Fields + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `flying` | [`Flying`](#flying) | A flying thing. | + | `foo` | [`UserOrGroup`](#userorgroup) | The foo. | + DOC + + union_section = <<~DOC + #### `UserOrGroup` + + Either a user or a group. + + One of: + + - [`Group`](#group) + - [`User`](#user) + DOC + + interface_section = <<~DOC + #### `Flying` + + Something that can fly. + + Implementations: + + - [`AfricanSwallow`](#africanswallow) + + ##### Fields + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `flightSpeed` | [`Int`](#int) | Speed in mph. | + DOC + + implementation_section = <<~DOC + ### `AfricanSwallow` + + A swallow from Africa. + + #### Fields + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `flightSpeed` | [`Int`](#int) | Speed in mph. | + DOC + + is_expected.to include( + type_section, + union_section, + interface_section, + implementation_section + ) + end + end + end +end -- cgit v1.2.3