sourcegraph / sourcegraph-public-snapshot

Code AI platform with Code Search & Cody
https://sourcegraph.com
Other
10.1k stars 1.27k forks source link

Support GitLab in campaigns #11586

Closed mrnugget closed 4 years ago

mrnugget commented 4 years ago

Steps:

At this point, we should be able to create changesets and display them (but their External* fields should be empty).

Next step is to support ChangesetEvents:

daxmc99 commented 4 years ago

Dear all,

This is your release captain speaking. 🚂🚂🚂

Branch cut for the 3.18 release is scheduled for tomorrow.

Is this issue / PR going to make it in time? Please change the milestone accordingly. When in doubt, reach out!

Thank you

mrnugget commented 4 years ago

@LawnGnome Do we want to create a separate ticket to track webhook support?

LawnGnome commented 4 years ago

@LawnGnome Do we want to create a separate ticket to track webhook support?

12171

mrnugget commented 4 years ago

@LawnGnome Something that came to my mind this morning: do we have any integration-y tests for GitLab? As far as I can tell in #11757 the GitLab client is mocked, so we don't record request/response in cassettes.

In our resolver tests we do have this one here, TestCampaigns that tracks a Bitbucket Server and a GitHub changeset and makes sure that the changeset resolvers work. But in #11675 I already ripped out that part, because the createChangesets mutation won't exist anymore.

I'm thinking that maybe we should extract that part of the test and create a TestChangesetResolver test that uses VCR-recorded and the rest of our stack to make sure that a fully-synced changeset from GH/GL/BBS works with the ChangesetResolver. What do you think?

The other test that I thought about it is this one: https://github.com/sourcegraph/sourcegraph/blob/870221e07c86de5e67d3900a804ce6547b595fd3/enterprise/internal/campaigns/resolvers/resolver_test.go#L651

That test currently only tests GitHub changesets on an integration-y level, but since changesetCountsOverTime, which is used for the burndown chart, has been tricky in the past, I wonder whether we shouldn't extend that.

And now to sum all of this up: I think we should have a test somewhere that makes sure that the flow of actual-data-from-gitLab->Sourcegraph->Sourcegraph-GraphQL-API works, but I leave it up to you to decide where/how/when to add this 😄