Skip to content
Snippets Groups Projects

Add contract test for suggested reviewers dropdown

Merged Mark Lapierre requested to merge ml-suggested-reviewer-contract-test into master
All threads resolved!

What does this MR do and why?

These changes were initially merged in !100653 (merged) and were reverted in !105198 (merged) after @leipert discovered (internal - Slack) that the Pact dependencies send usage data via Google Analytics.

This MR includes the original MR, plus opting out of sending usage data. Even if it's benign, we don't want to enroll engineers and users in sending usage data without their consent.

It also removes the dependencies that were added to package.json in the original MR. They're also included spec/contracts/consumer/package.json where they won't cause conflicts. The jest:contract script added to package.json can still be used to run contract tests from the root directory, as long as the packages in spec/contracts/consumer/package.json have been installed first.

Background

Contract tests were added recently and there is ongoing work.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Mark Lapierre

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Mark Lapierre changed milestone to %15.7

    changed milestone to %15.7

  • Mark Lapierre marked this merge request as ready

    marked this merge request as ready

  • 2 Messages
    :book: CHANGELOG missing:

    If you want to create a changelog entry for GitLab FOSS, add the Changelog trailer to the commit message you want to add to the changelog.

    If you want to create a changelog entry for GitLab EE, also add the EE: true trailer to your commit message.

    If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.

    :book: This merge request adds or changes documentation files. A review from the Technical Writing team before you merge is recommended. Reviews can happen after you merge.

    Documentation review

    The following files require a review from a technical writer:

    The review does not need to block merging this merge request. See the:

    Reviewer roulette

    Changes that require review have been detected!

    Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:

    Category Reviewer Maintainer
    backend Ian Anderson current availability (@imand3r) (UTC-8, 19 hours behind @mlapierre) Doug Stull current availability (@dstull) (UTC-5, 16 hours behind @mlapierre)
    frontend Doug Stull current availability (@dstull) (UTC-5, 16 hours behind @mlapierre) Coung Ngo current availability (@cngo) (UTC+0, 11 hours behind @mlapierre)
    test for spec/features/* Ian Anderson current availability (@imand3r) (UTC-8, 19 hours behind @mlapierre) Maintainer review is optional for test for spec/features/*

    To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

    To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

    Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.

    If needed, you can retry the :repeat: danger-review job that generated this comment.

    Generated by :no_entry_sign: Danger

  • Bundle size analysis [beta]

    This compares changes in bundle size for entry points between the commits ee6007b2 and 1be428b5

    :sparkles: Special assets

    Entrypoint / Name Size before Size after Diff Diff in percent
    average 3.52 MB 3.52 MB - 0.0 %
    mainChunk 1.95 MB 1.95 MB - 0.0 %

    Please look at the full report for more details


    Read more about how this report works.

    Generated by :no_entry_sign: Danger

  • Allure report

    allure-report-publisher generated test report!

    e2e-review-qa: :exclamation: test report for 1be428b5

    expand test summary
    +-----------------------------------------------------------------------------------------+
    |                                     suites summary                                      |
    +------------------------------------+--------+--------+---------+-------+-------+--------+
    |                                    | passed | failed | skipped | flaky | total | result |
    +------------------------------------+--------+--------+---------+-------+-------+--------+
    | Verify                             | 12     | 0      | 1       | 0     | 13    | ✅     |
    | Create                             | 28     | 0      | 1       | 0     | 29    | ✅     |
    | Plan                               | 49     | 0      | 1       | 0     | 50    | ✅     |
    | Manage                             | 39     | 0      | 4       | 6     | 43    | ❗     |
    | Configure                          | 0      | 0      | 1       | 0     | 1     | ➖     |
    | Govern                             | 15     | 0      | 5       | 3     | 20    | ❗     |
    | Package                            | 0      | 0      | 1       | 0     | 1     | ➖     |
    | Feature flag handler sanity checks | 9      | 0      | 0       | 0     | 9     | ✅     |
    | Version sanity check               | 0      | 0      | 1       | 0     | 1     | ➖     |
    +------------------------------------+--------+--------+---------+-------+-------+--------+
    | Total                              | 152    | 0      | 15      | 9     | 167   | ❗     |
    +------------------------------------+--------+--------+---------+-------+-------+--------+

    e2e-package-and-test: :exclamation: test report for 1be428b5

    expand test summary
    +---------------------------------------------------------------------------+
    |                              suites summary                               |
    +----------------------+--------+--------+---------+-------+-------+--------+
    |                      | passed | failed | skipped | flaky | total | result |
    +----------------------+--------+--------+---------+-------+-------+--------+
    | Analytics            | 11     | 0      | 0       | 0     | 11    | ✅     |
    | Manage               | 361    | 0      | 27      | 13    | 388   | ❗     |
    | Plan                 | 312    | 0      | 0       | 0     | 312   | ✅     |
    | Verify               | 249    | 0      | 17      | 5     | 266   | ❗     |
    | Govern               | 214    | 0      | 0       | 0     | 214   | ✅     |
    | Create               | 818    | 0      | 27      | 5     | 845   | ❗     |
    | Release              | 30     | 0      | 0       | 0     | 30    | ✅     |
    | Package              | 144    | 0      | 37      | 0     | 181   | ✅     |
    | Fulfillment          | 10     | 0      | 70      | 0     | 80    | ✅     |
    | Secure               | 35     | 0      | 5       | 20    | 40    | ❗     |
    | Monitor              | 5      | 0      | 0       | 0     | 5     | ✅     |
    | Configure            | 1      | 0      | 15      | 0     | 16    | ✅     |
    | Version sanity check | 0      | 0      | 7       | 7     | 7     | ➖     |
    | Systems              | 3      | 0      | 0       | 0     | 3     | ✅     |
    | GitLab Metrics       | 2      | 0      | 1       | 0     | 3     | ✅     |
    | Data Stores          | 13     | 0      | 1       | 0     | 14    | ✅     |
    | ModelOps             | 0      | 0      | 5       | 0     | 5     | ➖     |
    +----------------------+--------+--------+---------+-------+-------+--------+
    | Total                | 2208   | 0      | 212     | 50    | 2420  | ❗     |
    +----------------------+--------+--------+---------+-------+-------+--------+
  • Lukas Eipert approved this merge request

    approved this merge request

  • :wave: @leipert, thanks for approving this merge request.

    This is the first time the merge request is approved. To ensure full test coverage, a new pipeline will be started shortly.

    For more info, please refer to the following links:

  • 🤖 GitLab Bot 🤖 added 1 deleted label

    added 1 deleted label

  • Lukas Eipert
  • Lukas Eipert unapproved this merge request

    unapproved this merge request

  • mentioned in issue #380899 (closed)

  • Richard Chong mentioned in merge request !104892 (merged)

    mentioned in merge request !104892 (merged)

  • Mark Lapierre added 253 commits

    added 253 commits

    Compare with previous version

  • A deleted user added documentation label

    added documentation label

  • Mark Lapierre added 1 commit

    added 1 commit

    • d2292ee3 - Remove dependencies from root package.json

    Compare with previous version

  • Mark Lapierre added 1 commit

    added 1 commit

    • f3d30238 - Remove dependencies from root package.json

    Compare with previous version

  • mentioned in issue #364126 (closed)

  • Mark Lapierre requested review from @leipert

    requested review from @leipert

  • Lukas Eipert approved this merge request

    approved this merge request

  • Mark Lapierre added 2354 commits

    added 2354 commits

    Compare with previous version

  • Mark Lapierre added 1 commit

    added 1 commit

    • 1be428b5 - Remove dependencies from root package.json

    Compare with previous version

  • Mark Lapierre marked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed

    marked the checklist item I have evaluated the MR acceptance checklist for this MR. as completed

  • Mark Lapierre changed the description

    changed the description

    • Author Maintainer
      Resolved by Tristan Read

      @tristan.read Could you please review/merge this MR as maintainer? I've skipped getting reviews from other than frontend because the majority of the changes are the same as when initially merged in !100653 (merged)

      Just to clarify, the first commit is the same as was merged in !100653 (merged), and the two commits added in this MR opt out of Pact telemetry, and remove the dependencies that were added to package.json. It's only the last two commits that require review so the task is smaller than it seems :smile:

  • Mark Lapierre requested review from @tristan.read and removed review request for @leipert

    requested review from @tristan.read and removed review request for @leipert

  • @mlapierre This looks good to me, I'll approve. I followed the readme instructions and had some trouble getting one of the contract tests to pass locally, but the rest are green and the pipeline is green so I'll assume it's a local gdk issue.

    Setting MWPS :thumbsup_tone1:

  • Tristan Read approved this merge request

    approved this merge request

  • Tristan Read resolved all threads

    resolved all threads

  • Tristan Read enabled an automatic merge when the pipeline for 13b89b03 succeeds

    enabled an automatic merge when the pipeline for 13b89b03 succeeds

  • merged

  • Tristan Read mentioned in commit 0d454e4e

    mentioned in commit 0d454e4e

  • added workflowstaging label and removed workflowcanary label

  • Please register or sign in to reply
    Loading