From 135087430b3f6e49acc0b04ec0de98ba4bebfc82 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Mon, 25 Nov 2024 18:38:43 -0800
Subject: [PATCH] Make ThankYouEditTest better test updates in web request mode

CLI mode assertions will be enabled once core is updated in order
to avoid circular CI dependencies.

Rename getEditCount() to getPostSaveEditCount() and always add 1
to the count ahead of I50aa9fe9387c9b7b7ff97dfd39a2830bce647db8.
It will be off by 1 in CLI mode before that gets merged, which is
not likely to matter since most edits are via web requests. Even
edits through jobs are wrapped in the same DBO_TRX transactions as
web-requests for WMF sites since we use an HTTP-based job runner.

Change-Id: I33d94c1c54bc266c74c980ef1c73fd99a55c268e
---
 includes/Hooks.php                 | 15 ++++----
 tests/phpunit/ThankYouEditTest.php | 58 ++++++++++++++++++++++++++++--
 2 files changed, 62 insertions(+), 11 deletions(-)

diff --git a/includes/Hooks.php b/includes/Hooks.php
index 785a9719b..900e073ea 100644
--- a/includes/Hooks.php
+++ b/includes/Hooks.php
@@ -596,7 +596,7 @@ class Hooks implements
 		// test for them reaching a congratulatory threshold
 		$thresholds = [ 1, 10, 100, 1000, 10000, 100000, 1000000, 10000000 ];
 		if ( $userIdentity->isRegistered() ) {
-			$thresholdCount = $this->getEditCount( $userIdentity );
+			$thresholdCount = $this->getPredictedEditCount( $userIdentity );
 			if ( in_array( $thresholdCount, $thresholds ) ) {
 				DeferredUpdates::addCallableUpdate( static function () use (
 					$revisionRecord, $userIdentity, $title, $thresholdCount
@@ -664,17 +664,16 @@ class Hooks implements
 	}
 
 	/**
+	 * Get the predicted edit count after a page save hook
+	 *
 	 * @param UserIdentity $user
 	 * @return int
 	 */
-	private function getEditCount( UserIdentity $user ) {
+	private function getPredictedEditCount( UserIdentity $user ) {
 		$editCount = $this->userEditTracker->getUserEditCount( $user ) ?: 0;
-		// When this code runs from a maintenance script or unit tests
-		// the deferred update incrementing edit count runs right away
-		// so the edit count is right. Otherwise it lags by one.
-		if ( wfIsCLI() ) {
-			return $editCount;
-		}
+		// When this code runs, the deferred update that increments the edit count
+		// will still be pending.
+
 		return $editCount + 1;
 	}
 
diff --git a/tests/phpunit/ThankYouEditTest.php b/tests/phpunit/ThankYouEditTest.php
index 027f709e1..079cdeddc 100644
--- a/tests/phpunit/ThankYouEditTest.php
+++ b/tests/phpunit/ThankYouEditTest.php
@@ -5,6 +5,7 @@ namespace MediaWiki\Extension\Notifications\Test;
 use MediaWiki\Deferred\DeferredUpdates;
 use MediaWiki\Extension\Notifications\DbFactory;
 use MediaWiki\Extension\Notifications\Mapper\NotificationMapper;
+use MediaWiki\Extension\Notifications\Model\Notification;
 use MediaWiki\Title\Title;
 use MediaWikiIntegrationTestCase;
 use Wikimedia\Rdbms\Platform\ISQLPlatform;
@@ -29,17 +30,43 @@ class ThankYouEditTest extends MediaWikiIntegrationTestCase {
 			->execute();
 	}
 
+	public function provideFirstEditRequestModes() {
+		return [
+			[ 'web' ],
+			[ 'cli' ]
+		];
+	}
+
 	/**
 	 * @covers \MediaWiki\Extension\Notifications\Hooks::onPageSaveComplete
+	 * @dataProvider provideFirstEditRequestModes
+	 * @param string $mode
 	 */
-	public function testFirstEdit() {
+	public function testFirstEdit( $mode ) {
+		// TODO: re-renable once I50aa9fe9387c9b7b7ff97dfd39a2830bce647db8 is merged.
+		// That is, after, endAtomic() in PageUpdater::doCreate() is tweaked
+		if ( $mode === 'cli' ) {
+			$this->markTestSkipped();
+		}
+
 		// setup
 		$this->deleteEchoData();
 		$user = $this->getMutableTestUser()->getUser();
 		$title = Title::newFromText( 'Help:MWEchoThankYouEditTest_testFirstEdit' );
 
 		// action
+		$db = $this->getDb();
+		// Web requests wrap the edit in a broader transaction via DBO_TRX and commit it in
+		// MediaWikiEntryPoint::commitMainTransaction(). We can largely simulate that by just
+		// using atomic sections.
+		$useAtomicSection = ( $mode === 'web' );
+		if ( $useAtomicSection ) {
+			$db->startAtomic( __METHOD__ );
+		}
 		$this->editPage( $title, 'this is my first edit', '', NS_MAIN, $user );
+		if ( $useAtomicSection ) {
+			$db->endAtomic( __METHOD__ );
+		}
 		DeferredUpdates::tryOpportunisticExecute();
 
 		// assertions
@@ -52,10 +79,24 @@ class ThankYouEditTest extends MediaWikiIntegrationTestCase {
 		$this->assertSame( 1, $notification->getEvent()->getExtraParam( 'editCount', 'not found' ) );
 	}
 
+	public function provideTenthEditRequestModes() {
+		return [
+			[ 'web' ],
+			[ 'cli' ]
+		];
+	}
+
 	/**
 	 * @covers \MediaWiki\Extension\Notifications\Hooks::onPageSaveComplete
+	 * @dataProvider provideTenthEditRequestModes
+	 * @param string $mode
 	 */
-	public function testTenthEdit() {
+	public function testTenthEdit( $mode ) {
+		// TODO: re-renable once endAtomic() in PageUpdater::doCreate() is tweaked
+		if ( $mode === 'cli' ) {
+			$this->markTestSkipped();
+		}
+
 		// setup
 		$this->deleteEchoData();
 		$user = $this->getMutableTestUser()->getUser();
@@ -66,10 +107,21 @@ class ThankYouEditTest extends MediaWikiIntegrationTestCase {
 		// we could fast-forward the edit-count to speed things up
 		// but this is the only way to make sure duplicate notifications
 		// are not generated
+		$db = $this->getDb();
+		// Web requests wrap the edit in a broader transaction via DBO_TRX and commit it in
+		// MediaWikiEntryPoint::commitMainTransaction(). We can largely simulate that by just
+		// using atomic sections.
+		$useAtomicSection = ( $mode === 'web' );
 		for ( $i = 0; $i < 12; $i++ ) {
+			if ( $useAtomicSection ) {
+				$db->startAtomic( __METHOD__ );
+			}
 			$this->editPage( $page, "this is edit #$i", '', NS_MAIN, $user );
+			if ( $useAtomicSection ) {
+				$db->endAtomic( __METHOD__ );
+			}
+			DeferredUpdates::tryOpportunisticExecute();
 		}
-		DeferredUpdates::tryOpportunisticExecute();
 		$user->clearInstanceCache();
 
 		// assertions