Skip to content
Snippets Groups Projects

[QA] Use public_email instead of email since it's available

Merged Rémy Coutable requested to merge qa-staging-27 into master
All threads resolved!

What does this MR do?

This makes sure we fetch public_email from the response API instead of email which is only available to admins.

This also add tests for the User resource.

What are the relevant issue numbers?

Closes gitlab-org/quality/staging#27 (closed).

Does this MR meet the acceptance criteria?

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
  • Thanks for making these changes @rymai! I left a few minor comments, mostly really just nit-picks. But the test also failed, presumably because public_email isn't actually set for the root user.

  • assigned to @rymai

  • Rémy Coutable added 42 commits

    added 42 commits

    Compare with previous version

  • Rémy Coutable resolved all discussions

    resolved all discussions

  • Author Maintainer

    @mlapierre Thanks for the review, I think I addressed everything, I've started the review-qa-all job here: https://gitlab.com/gitlab-org/gitlab-ce/-/jobs/149344796 :fingers_crossed:

  • @rymai So close!

     2) Create Commit data user views raw email patch
         Failure/Error: expect(page).to have_content("From: #{user.name} <#{user.public_email}>")
           expected to find text "From: Administrator <root@example.com>" in "From c7483faa94b32bc5afae9862eca19edd6288727d Mon Sep 17 00:00:00 2001 From: Administrator <admin@example.com> Date: Tue, 22 Jan 2019 14:40:41 +0000 Subject: [PATCH] Add second file --- second | 1 + 1 file changed, 1 insertion(+) create mode 100644 second diff --git a/second b/second new file mode 100644 index 0000000..df514d3 --- /dev/null +++ b/second @@ -0,0 +1 @@ +second file content \\ No newline at end of file -- 2.18.1"
         # ./qa/specs/features/browser_ui/3_create/repository/user_views_raw_diff_patch_requests_spec.rb:50:in `block (3 levels) in <module:QA>'

    Hmm, I didn't notice before that the expected email address is admin@example.com. That doesn't seem right?

  • assigned to @rymai

  • @rymai The review app isn't responding now and some of the workloads are showing errors or warnings. Do you know if it's possible to restart those workloads, or would we need to restart the whole review-deploy job?

  • @rymai FYI I restarted the review-deploy job. But if there is a better way I'd be keen to try that first next time.

  • Author Maintainer

    The review app isn't responding now and some of the workloads are showing errors or warnings. Do you know if it's possible to restart those workloads, or would we need to restart the whole review-deploy job?

    Yes, sorry, I've changed the node pools configuration because some pods weren't schedulable, that may be the cause...

    FYI I restarted the review-deploy job. But if there is a better way I'd be keen to try that first next time.

    Yeah, that should solve it. :thumbsup:

  • Author Maintainer

    Hmm, I didn't notice before that the expected email address is admin@example.com. That doesn't seem right?

    @mlapierre That's the default in https://gitlab.com/gitlab-org/gitlab-ce/blob/7363428c0928b14bfd8c85a2a16d0f36622db747/db/fixtures/production/002_admin.rb#L2 so I guess we should make it the default in QA too... I think I will add default_email to QA::Runtime::User and default to that in Resource::User, WDYT?

  • That's the default in https://gitlab.com/gitlab-org/gitlab-ce/blob/7363428c0928b14bfd8c85a2a16d0f36622db747/db/fixtures/production/002_admin.rb#L2 so I guess we should make it the default in QA too... I think I will add default_email to QA::Runtime::User and default to that in Resource::User, WDYT?

    Yup, sounds good to me! :thumbsup: ...and I guess admin is more common in an email address than root, so it does make sense.

  • Rémy Coutable added 43 commits

    added 43 commits

    Compare with previous version

  • Author Maintainer

    @mlapierre I've implemented Runtime::User.default_email as discussed. Please have another look!

  • assigned to @rymai

  • Rémy Coutable added 46 commits

    added 46 commits

    Compare with previous version

  • Rémy Coutable added 5 commits

    added 5 commits

    Compare with previous version

  • Author Maintainer

    @mlapierre QA is :white_check_mark: (the failure is unrelated). Please have another look, thanks!

  • Looks good, thanks @rymai! :thumbsup: :rocket:

  • Mark Lapierre approved this merge request

    approved this merge request

  • merged

  • Mark Lapierre mentioned in commit 19add921

    mentioned in commit 19add921

  • Sanad Liaquat changed milestone to %11.7

    changed milestone to %11.7

  • Sanad Liaquat added 1 deleted label

    added 1 deleted label

  • Mark Lapierre mentioned in commit 58ec656b

    mentioned in commit 58ec656b

  • Automatically picked into https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24941, will merge into 11-7-stable ready for 11.7.5.

  • Mark Lapierre mentioned in commit cf47f170

    mentioned in commit cf47f170

  • GitLab Release Tools Bot removed 1 deleted label

    removed 1 deleted label

  • mentioned in merge request !24941 (merged)

  • Please register or sign in to reply
    Loading