EBernhardson has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/396552 )

Change subject: Port update_weight_api.feature to nodejs
......................................................................

Port update_weight_api.feature to nodejs

Replace 'within ...' calls with a new step that waits for incoming_links
to be appropriately updated. This makes our expectations explicit
instead of waiting on a secondary thing that hopefully updates based on
the first. Additionally to save some time (this test is pretty slow)
skip waiting on most of the edits and just do the final wait for
incoming_links.

I'm not really sure these tests even really need to be performing
searches, it looks like mostly they are checking that incoming links are
updated and counted appropriately.

While working up this patch i noticed multiple steps are all now
utilizing a 'wait for elasticsearch document to have some value' type
step so moved the implementation into stepHelpers and adjusted the uses
to all have similar wording and use the same implementation with
different check functions.

Change-Id: I20b13236e2139026d542de5e376392d6c5a67e47
---
M tests/integration/features/commons.feature
M tests/integration/features/step_definitions/page_step_helpers.js
M tests/integration/features/step_definitions/page_steps.js
M tests/integration/features/update_redirect_api.feature
A tests/integration/features/update_weight_api.feature
5 files changed, 142 insertions(+), 43 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CirrusSearch 
refs/changes/52/396552/1

diff --git a/tests/integration/features/commons.feature 
b/tests/integration/features/commons.feature
index b36867b..764f74d 100644
--- a/tests/integration/features/commons.feature
+++ b/tests/integration/features/commons.feature
@@ -5,7 +5,7 @@
     Then File:OnCommons.svg is the first api search result
 
   Scenario: A file that exists on commons and the local wiki returns the local 
result
-    When within 20 seconds File:DuplicatedLocally.svg has cirrustestwiki as 
local_sites_with_dupe
+    When I wait for File:DuplicatedLocally.svg on commons to include 
cirrustestwiki in local_sites_with_dupe
     Then I api search in namespace 6 for duplicated
     Then File:DuplicatedLocally.svg is the first api search result
     And Locally stored file *duplicated* on commons is the highlighted snippet 
of the first api search result
diff --git a/tests/integration/features/step_definitions/page_step_helpers.js 
b/tests/integration/features/step_definitions/page_step_helpers.js
index b5f39cc..0979e90 100644
--- a/tests/integration/features/step_definitions/page_step_helpers.js
+++ b/tests/integration/features/step_definitions/page_step_helpers.js
@@ -171,6 +171,29 @@
                } ).call( this );
        }
 
+       waitForDocument( title, check ) {
+               return Promise.coroutine( function* () {
+                       let timeoutMs = 20000;
+                       let start = new Date();
+                       let lastError;
+                       while ( true ) {
+                               let doc = yield this.getCirrusIndexedContent( 
title );
+                               if ( doc.cirrusdoc && doc.cirrusdoc.length > 0 
) {
+                                       try {
+                                               check( doc.cirrusdoc[0] );
+                                               break;
+                                       } catch ( err ) {
+                                               lastError = err;
+                                       }
+                               }
+                               if ( new Date() - start >= timeoutMs ) {
+                                       throw lastError || new Error( `Timeout 
out waiting for ${title}` );
+                               }
+                               yield this.waitForMs( 200 );
+                       }
+               } ).call( this );
+       }
+
        waitForMs( ms ) {
                return new Promise( ( resolve ) => setTimeout( resolve, ms ) );
        }
diff --git a/tests/integration/features/step_definitions/page_steps.js 
b/tests/integration/features/step_definitions/page_steps.js
index f3d7a83..bb3eec6 100644
--- a/tests/integration/features/step_definitions/page_steps.js
+++ b/tests/integration/features/step_definitions/page_steps.js
@@ -386,27 +386,6 @@
                return stepHelpers.uploadFile( title, fileName, description );
        } );
 
-       Then(/^within (\d+) seconds (.+) has (.+) as local_sites_with_dupe$/, 
function (seconds, title, value) {
-               return Promise.coroutine( function* () {
-                       let stepHelpers = this.stepHelpers.onWiki( 'commons' );
-                       let time = new Date();
-                       let found = false;
-                       main: do {
-                               let page = yield 
stepHelpers.getCirrusIndexedContent( title );
-                               if ( page ) {
-                                       for ( let doc of page.cirrusdoc ) {
-                                               found = doc.source && 
doc.source.local_sites_with_dupe &&
-                                                       
doc.source.local_sites_with_dupe.indexOf( value ) > -1;
-                                               if ( found ) break main;
-                                       }
-                               }
-                               yield stepHelpers.waitForMs( 100 );
-                       } while ( ( new Date() - time ) < ( seconds * 1000 ) );
-                       let msg = `Expected ${title} on commons to have 
${value} in local_sites_with_dupe within ${seconds}s.`;
-                       expect( found, msg ).to.equal(true);
-               } ).call( this );
-       } );
-
        Then(/^I am on a page titled (.+)$/, function( title ) {
                expect(ArticlePage.articleTitle, `I am on 
${title}`).to.equal(title);
        } );
@@ -482,28 +461,39 @@
        } );
 
        Then( /^there are( no)? api search results with (.+) in the data$/, 
function ( should_not, within ) {
-               let snippets = this.apiResponse.query.search.map( ( result ) => 
result.snippet );
-               let found = snippets.reduce( ( a, b ) => a || b.indexOf( within 
) > -1, false );
-               expect( found ).to.equal( !should_not );
+               return withApi( this, () => {
+                       let snippets = this.apiResponse.query.search.map( ( 
result ) => result.snippet );
+                       let found = snippets.reduce( ( a, b ) => a || 
b.indexOf( within ) > -1, false );
+                       expect( found ).to.equal( !should_not );
+               } );
        } );
 
-       Then( /^I wait for (.+) to not be included in the redirects of (.+)$/, 
function ( source, redirect ) {
-               return Promise.coroutine( function* () {
-                       let timeoutMs = 20000;
-                       let start = new Date();
-                       while (true) {
-                               let doc = yield 
this.stepHelpers.getCirrusIndexedContent( redirect );
-                               if ( doc.cirrusdoc.length > 0 ) {
-                                       let exists = 
doc.cirrusdoc[0].source.redirect.reduce( ( a, b ) => a || b.title === source, 
false ); // jshint ignore:line
-                                       if ( !exists ) {
-                                               break;
-                                       }
-                               }
-                               if (new Date() - start >= timeoutMs) {
-                                       throw new Error( `Timed out waiting for 
${source} to not exist in document of ${redirect}` );
-                               }
-                               yield this.stepHelpers.waitForMs( 200 );
+       Then( /^I wait for (.+) to not include (.+) in redirects$/, function ( 
title, source ) {
+               return withApi( this, () => {
+                       return this.stepHelpers.waitForDocument( title, ( doc ) 
=> {
+                               let titles = doc.source.redirect.map( ( 
redirect ) => redirect.title );
+                               expect( titles ).to.not.include( source );
+                       } );
+               } );
+       } );
+
+       Then( /^I wait for (.+?)(?: on (.+))? to include (.+) in (.+)$/, 
function ( title, wiki, value, field ) {
+               return withApi( this, () => {
+                       let stepHelpers = this.stepHelpers;
+                       if ( wiki ) {
+                               stepHelpers = this.stepHelpers.onWiki( wiki );
                        }
-               } ).call( this );
+                       return stepHelpers.waitForDocument( title, ( doc ) => {
+                               expect( doc.source[field] ).to.include( value );
+                       } );
+               } );
+       } );
+
+       Then( /^I wait for (.+) to have (.+) of (.+)$/, function ( title, 
field, value ) {
+               return withApi( this, () => {
+                       return this.stepHelpers.waitForDocument( title, ( doc ) 
=> {
+                               expect( String( doc.source[field] ) ).to.equal( 
value );
+                       } );
+               } );
        } );
 });
diff --git a/tests/integration/features/update_redirect_api.feature 
b/tests/integration/features/update_redirect_api.feature
index 50171ea..b2340b6 100644
--- a/tests/integration/features/update_redirect_api.feature
+++ b/tests/integration/features/update_redirect_api.feature
@@ -16,7 +16,10 @@
       And I api search for StartsAsRedirect%{epoch}
      Then RedirectTarget is the first api search result
      When a page named StartsAsRedirect%{epoch} exists
-         And I wait for StartsAsRedirect%{epoch} to not be included in the 
redirects of RedirectTarget
+      And I wait for RedirectTarget to not include StartsAsRedirect%{epoch} in 
redirects
+      # Waiting for the redirect to become not-a-redirect doesn't seem to 
reliably wait. This
+      # is still not reliable ... but hopefully it helps.
+      And I wait 2 seconds
       And I api search for StartsAsRedirect%{epoch}
      Then StartsAsRedirect%{epoch} is the first api search result
       And RedirectTarget is not in the api search results
diff --git a/tests/integration/features/update_weight_api.feature 
b/tests/integration/features/update_weight_api.feature
new file mode 100644
index 0000000..2e6dbdb
--- /dev/null
+++ b/tests/integration/features/update_weight_api.feature
@@ -0,0 +1,83 @@
+@clean @api @update @weight
+Feature: Page updates trigger appropriate weight updates in newly linked and 
unlinked articles
+  # Note that these tests can be a bit flakey if you don't use Redis and 
checkDelay because they count using
+  # Elasticsearch which delays all updates for around a second.  So if the 
jobs run too fast they won't work.
+  # Redis and checkDelay fix this by forcing a delay.
+  Scenario: Pages weights are updated when new pages link to them
+    Given I don't wait for a page named WeightedLink%{epoch} 1 to exist
+      And I don't wait for a page named WeightedLink%{epoch} 2/1 to exist with 
contents [[WeightedLink%{epoch} 2]]
+      And I don't wait for a page named WeightedLink%{epoch} 2 to exist
+      And I wait for WeightedLink%{epoch} 2 to have incoming_links of 1
+      And I api search for WeightedLink%{epoch}
+     Then WeightedLink%{epoch} 2 is the first api search result
+     When I don't wait for a page named WeightedLink%{epoch} 1/1 to exist with 
contents [[WeightedLink%{epoch} 1]]
+      And I don't wait for a page named WeightedLink%{epoch} 1/2 to exist with 
contents [[WeightedLink%{epoch} 1]]
+      And I wait for WeightedLink%{epoch} 1 to have incoming_links of 2
+      And I api search for WeightedLink%{epoch}
+     Then WeightedLink%{epoch} 1 is the first api search result
+
+  @expect_failure
+  Scenario: Pages weights are updated when links are removed from them
+    Given I don't wait for a page named WeightedLinkRemoveUpdate%{epoch} 1/1 
to exist with contents [[WeightedLinkRemoveUpdate%{epoch} 1]]
+      And I don't wait for a page named WeightedLinkRemoveUpdate%{epoch} 1/2 
to exist with contents [[WeightedLinkRemoveUpdate%{epoch} 1]]
+      And I don't wait for a page named WeightedLinkRemoveUpdate%{epoch} 1 to 
exist
+      And I don't wait for a page named WeightedLinkRemoveUpdate%{epoch} 2/1 
to exist with contents [[WeightedLinkRemoveUpdate%{epoch} 2]]
+      And I don't wait for a page named WeightedLinkRemoveUpdate%{epoch} 2 to 
exist
+      And I wait for WeightedLinkRemoveUpdate%{epoch} 1 to have incoming_links 
of 2
+      And I wait for WeightedLinkRemoveUpdate%{epoch} 2 to have incoming_links 
of 1
+      And I api search for WeightedLinkRemoveUpdate%{epoch}
+     Then WeightedLinkRemoveUpdate%{epoch} 1 is the first api search result
+     When I don't wait for a page named WeightedLinkRemoveUpdate%{epoch} 1/1 
to exist with contents [[Junk]]
+      And I don't wait for a page named WeightedLinkRemoveUpdate%{epoch} 1/2 
to exist with contents [[Junk]]
+      And I wait for WeightedLinkRemoveUpdate%{epoch} 1 to have incoming_links 
of 0
+      And I api search for WeightedLinkRemoveUpdate%{epoch}
+     Then WeightedLinkRemoveUpdate%{epoch} 2 is the first api search result
+
+  Scenario: Pages weights are updated when new pages link to their redirects
+    Given I don't wait for a page named WeightedLinkRdir%{epoch} 1/Redirect to 
exist with contents #REDIRECT [[WeightedLinkRdir%{epoch} 1]]
+      And I don't wait for a page named WeightedLinkRdir%{epoch} 1 to exist
+      And I don't wait for a page named WeightedLinkRdir%{epoch} 2/Redirect to 
exist with contents #REDIRECT [[WeightedLinkRdir%{epoch} 2]]
+      And I don't wait for a page named WeightedLinkRdir%{epoch} 2/1 to exist 
with contents [[WeightedLinkRdir%{epoch} 2/Redirect]]
+      And I don't wait for a page named WeightedLinkRdir%{epoch} 2 to exist
+      And I wait for WeightedLinkRdir%{epoch} 1 to have incoming_links of 1
+      And I wait for WeightedLinkRdir%{epoch} 2 to have incoming_links of 2
+      And I api search for WeightedLinkRdir%{epoch}
+     Then WeightedLinkRdir%{epoch} 2 is the first api search result
+     When I don't wait for a page named WeightedLinkRdir%{epoch} 1/1 to exist 
with contents [[WeightedLinkRdir%{epoch} 1/Redirect]]
+      And I don't wait for a page named WeightedLinkRdir%{epoch} 1/2 to exist 
with contents [[WeightedLinkRdir%{epoch} 1/Redirect]]
+      And I wait for WeightedLinkRdir%{epoch} 1 to have incoming_links of 3
+      And I api search for WeightedLinkRdir%{epoch}
+     Then WeightedLinkRdir%{epoch} 1 is the first api search result
+
+  Scenario: Pages weights are updated when links are removed from their 
redirects
+    Given I don't wait for a page named WLRURdir%{epoch} 1/1 to exist with 
contents [[WLRURdir%{epoch} 1/Redirect]]
+      And I don't wait for a page named WLRURdir%{epoch} 1/2 to exist with 
contents [[WLRURdir%{epoch} 1/Redirect]]
+      And I don't wait for a page named WLRURdir%{epoch} 1/Redirect to exist 
with contents #REDIRECT [[WLRURdir%{epoch} 1]]
+      And I don't wait for a page named WLRURdir%{epoch} 1 to exist
+      And I don't wait for a page named WLRURdir%{epoch} 2/Redirect to exist 
with contents #REDIRECT [[WLRURdir%{epoch} 2]]
+      And I don't wait for a page named WLRURdir%{epoch} 2/1 to exist with 
contents [[WLRURdir%{epoch} 2/Redirect]]
+      And I don't wait for a page named WLRURdir%{epoch} 2 to exist
+      And I wait for WLRURdir%{epoch} 1 to have incoming_links of 3
+      And I wait for WLRURdir%{epoch} 2 to have incoming_links of 2
+      And I api search for WLRURdir%{epoch}
+     Then WLRURdir%{epoch} 1 is the first api search result
+     When I don't wait for a page named WLRURdir%{epoch} 1/1 to exist with 
contents [[Junk]]
+      And I don't wait for a page named WLRURdir%{epoch} 1/2 to exist with 
contents [[Junk]]
+      And I wait for WLRURdir%{epoch} 1 to have incoming_links of 1
+      And I api search for WLRURdir%{epoch}
+     Then WLRURdir%{epoch} 2 is the first api search result
+
+  Scenario: Redirects to redirects don't count in the score
+    Given I don't wait for a page named WLDoubleRdir%{epoch} 1/Redirect to 
exist with contents #REDIRECT [[WLDoubleRdir%{epoch} 1]]
+      And I don't wait for a page named WLDoubleRdir%{epoch} 1/Redirect 
Redirect to exist with contents #REDIRECT [[WLDoubleRdir%{epoch} 1/Redirect]]
+      And I don't wait for a page named WLDoubleRdir%{epoch} 1/1 to exist with 
contents [[WLDoubleRdir%{epoch} 1/Redirect Redirect]]
+      And I don't wait for a page named WLDoubleRdir%{epoch} 1/2 to exist with 
contents [[WLDoubleRdir%{epoch} 1/Redirect Redirect]]
+      And I don't wait for a page named WLDoubleRdir%{epoch} 1 to exist
+      And I don't wait for a page named WLDoubleRdir%{epoch} 2/Redirect to 
exist with contents #REDIRECT [[WLDoubleRdir%{epoch} 2]]
+      And I don't wait for a page named WLDoubleRdir%{epoch} 2/1 to exist with 
contents [[WLDoubleRdir%{epoch} 2/Redirect]]
+      And I don't wait for a page named WLDoubleRdir%{epoch} 2/2 to exist with 
contents [[WLDoubleRdir%{epoch} 2/Redirect]]
+      And I don't wait for a page named WLDoubleRdir%{epoch} 2 to exist
+      And I wait for WLDoubleRdir%{epoch} 1 to have incoming_links of 1
+      And I wait for WLDoubleRdir%{epoch} 2 to have incoming_links of 3
+      And I api search for WLDoubleRdir%{epoch}
+     Then WLDoubleRdir%{epoch} 2 is the first api search result

-- 
To view, visit https://gerrit.wikimedia.org/r/396552
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I20b13236e2139026d542de5e376392d6c5a67e47
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CirrusSearch
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to