Differential D807

Improve API replies handling in NotifyNewCommitsToDiffusion
ClosedPublic
Actions

Authored by dereckson on Jan 7 2017, 19:35.

Tags

None

Referenced Files

	Unknown Object (File)
	Fri, Dec 13, 19:56

	Unknown Object (File)
	Tue, Dec 10, 12:03

	Unknown Object (File)
	Tue, Dec 10, 09:52

	Unknown Object (File)
	Tue, Dec 10, 09:16

	Unknown Object (File)
	Tue, Dec 10, 09:01

	Unknown Object (File)
	Sat, Dec 7, 21:20

	Unknown Object (File)
	Thu, Dec 5, 17:21

	Unknown Object (File)
	Wed, Dec 4, 13:21

Subscribers

None

Details

Reviewers

Maniphest Tasks

T999: NotifyNewCommitsToDiffusion fails to check if we've or not a Phabricator instance for this project

Commits

rNOTIF6653ad57b95c: Improve API replies handling in NotifyNewCommitsToDiffusion

Summary

291a59a6 introduced exceptions in PhabricatorAPI static constructors.
Formerly, they returned null.

When a new payload comes from GitHub, the NotifyNewCommitsToDiffusion job
notifies Phabricator of the new commit. But when there is no Phabricator
instance for the same door (or in the future a mapping, see T851),
the code doesn't detect this new exception thrown.

Fixes T999.

Test Plan

Improve NotifyNewCommitsToDiffusionTest to correctly
mock Phabricator API replies, and test each scenarii.

Diff Detail

Repository

rNOTIF Notifications center

Lint

Lint Passed

Unit

Tests Passed

Branch

T999 (branched from master)

Build Status

Buildable 1239
Build 1487: arc lint + arc unit

Event Timeline

dereckson created this revision.Jan 7 2017, 19:35

Quick test for no repo but switch to a provider

dereckson planned changes to this revision.Jan 7 2017, 19:41

Simplify test, dataProvider

dereckson accepted this revision.Jan 7 2017, 20:33

dereckson retitled this revision from Fix NotifyNewCommitsToDiffusion to Improve API replies handling in NotifyNewCommitsToDiffusion.

dereckson edited the test plan for this revision. (Show Details)

This revision is now accepted and ready to land.Jan 7 2017, 20:34

Closed by commit rNOTIF6653ad57b95c: Improve API replies handling in NotifyNewCommitsToDiffusion (authored by dereckson, committed by dereckson). · Explain WhyJan 7 2017, 20:35

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

app/

Jobs/

NotifyNewCommitsToDiffusion.php

11 lines

tests/

Jobs/

NotifyNewCommitsToDiffusionTest.php

70 lines

Diff 2046

app/Jobs/NotifyNewCommitsToDiffusion.php

Loading...

tests/Jobs/NotifyNewCommitsToDiffusionTest.php

Loading...