Skip to content

Conversation

@lucila
Copy link
Contributor

@lucila lucila commented Oct 24, 2025

See: #41

What and why? 🤔

We want to maje StreamBuilder compatible with PHP 8.4.

Note: this is a breaking change because the order of the constructor params has changed.
When creating a new build, let's update the major version

Added coverage for mutants in Capped Post Ranker

Updated --only-covered for --only-covering-test-cases

Testing Steps ✍️

code review should be enough

You're it! 👑

Tag a random reviewer by adding @Automattic/stream-builders to the reviewers section, but mention them here for visibility as well.

@lucila lucila requested a review from a team as a code owner October 24, 2025 16:27
@lucila lucila requested review from koke and markbiek October 24, 2025 16:27
@coveralls
Copy link

coveralls commented Oct 24, 2025

Coverage Status

coverage: 80.223%. remained the same
when pulling 38953e3 on lucila/update-cappedpostranker
into c77da04 on main.

@lucila
Copy link
Contributor Author

lucila commented Oct 24, 2025

I'm getting errors not related to this change:

% vendor/bin/infection --coverage=build/logs --threads=$(nproc) --show-mutations --no-interaction --only-covered --only-covering-test-cases --skip-initial-tests

    ____      ____          __  _
   /  _/___  / __/__  _____/ /_(_)___  ____
   / // __ \/ /_/ _ \/ ___/ __/ / __ \/ __ \
 _/ // / / / __/  __/ /__/ /_/ / /_/ / / / /
/___/_/ /_/_/  \___/\___/\__/_/\____/_/ /_/

#StandWithUkraine

Infection - PHP Mutation Testing Framework version 0.29.8

[warning] Skipping the initial test run can be very dangerous.
It is your responsibility to ensure the tests are in a passing state to begin.
If this is not done then mutations may report as caught when they are not.


Generate mutants...

Processing source code files: 142/142
.: killed, M: escaped, U: uncovered, E: fatal error, X: syntax error, T: timed out, S: skipped, I: ignored

IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII   ( 50 / 155)
IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII   (100 / 155)
IIIIIIIIIIIIIIIIIIII.............M.MMM..M.M....M..   (150 / 155)
MM..M                                                (155 / 155)
Escaped mutants:
================


1) /Users/lucilastancato/Documents/Tumblr/stream-builder/lib/Tumblr/StreamBuilder/StreamElementInjection.php:43    [M] MethodCallRemoval

@@ @@
         parent::__construct($injector);
         $this->element = $element;
         if (empty($element->getComponent())) {
-            $this->element->setComponent($injector->getComponent());
+            
         }
     }
     /**


2) /Users/lucilastancato/Documents/Tumblr/stream-builder/lib/Tumblr/StreamBuilder/StreamFilters/ChronologicalRangeFilter.php:76    [M] MethodCallRemoval

@@ @@
     #[\Override]
     protected function pre_fetch(array $elements)
     {
-        StreamElement::pre_fetch_all($elements);
+        
     }
     /**
      * @inheritDoc


3) /Users/lucilastancato/Documents/Tumblr/stream-builder/lib/Tumblr/StreamBuilder/StreamFilters/StreamElementFilter.php:43    [M] MethodCallRemoval

@@ @@
     {
         $retained = [];
         $released = [];
-        $this->pre_fetch($elements);
+        
         foreach ($elements as $e) {
             if ($this->should_release($e)) {
                 $released[] = $e;


4) /Users/lucilastancato/Documents/Tumblr/stream-builder/lib/Tumblr/StreamBuilder/StreamInjectors/GeneralStreamInjector.php:117    [M] MethodCallRemoval

@@ @@
         }
         $new_state = $this->cursorToInjectionState($injection_contents->get_combined_cursor(), $allocate_result->get_injector_state() ?? []);
         $plan = array_map(function (StreamElementInjection $p) {
-            $p->get_element()->set_cursor(null);
+            
             return $p;
         }, $plan);
         return new InjectionPlan($plan, $new_state);


5) /Users/lucilastancato/Documents/Tumblr/stream-builder/lib/Tumblr/StreamBuilder/StreamRankers/CappedPostRanker.php:195    [M] MethodCallRemoval

@@ @@
                 // No violation: 1. Copy element to new list of ranked elements and 2. Increase num of shown posts
                 array_push($ranked_elements, $post);
                 $post_added[$current_post_id] = true;
-                $this->invalidate_post_id($current_blog_id, $current_post_id, $blog_to_posts_ids);
+                
                 $this->bump_seen_posts($blog_to_stats, $current_blog_id);
             } else {
                 // Violation: 1. Fetch and add the violated post instead, 2. Save current post for later


6) /Users/lucilastancato/Documents/Tumblr/stream-builder/lib/Tumblr/StreamBuilder/StreamRankers/CappedPostRanker.php:224    [M] MethodCallRemoval

@@ @@
                 // Add the violator
                 array_push($ranked_elements, $post_to_element[$current_post_id]);
                 $post_added[$current_post_id] = true;
-                $this->invalidate_post_id($current_blog_id, $current_post_id, $blog_to_posts_ids);
+                
                 $this->bump_seen_posts($blog_to_stats, $current_blog_id);
                 if ($debug) {
                     $post_to_element[$current_post_id]->add_debug_info('capped_reranking', 'capped', true);


7) /Users/lucilastancato/Documents/Tumblr/stream-builder/lib/Tumblr/StreamBuilder/StreamRankers/StreamRanker.php:58    [M] MethodCallRemoval

@@ @@
             $tracer->begin_rank($this, $t0);
         }
         try {
-            // note: pre_fetch is counted inside the rank() timer!
-            $this->pre_fetch($stream_elements);
+            
             $ranked = $this->rank_inner($stream_elements, $tracer);
             if (!Helpers::verify_reordered_elements($stream_elements, $ranked)) {
                 // we throw this in here so it will trigger fail_rank to be called on the tracer.


8) /Users/lucilastancato/Documents/Tumblr/stream-builder/lib/Tumblr/StreamBuilder/StreamSerializer.php:61    [M] MethodCallRemoval

@@ @@
         /** @var Templatable $object */
         $object = call_user_func([$template['_type'], 'from_template'], $context);
         $component = $template[StreamContext::COMPONENT_NAME] ?? null;
-        $object->setComponent($component);
+        
         $skip_components = $context->get_meta_by_key(StreamContext::SKIP_COMPONENT_META) ?? [];
         if (in_array($component, $skip_components, true)) {
             $object->setSkippedComponent(true);


9) /Users/lucilastancato/Documents/Tumblr/stream-builder/lib/Tumblr/StreamBuilder/Streams/ChronologicalStreamMixer.php:106    [M] MethodCallRemoval

@@ @@
                 }
             }
         }
-        // magic pre-fetcher!
-        StreamElement::pre_fetch_all($res);
+        
         usort($res, function (StreamElement $el_1, StreamElement $el_2) {
             $el_1 = $el_1->get_original_element();
             $el_2 = $el_2->get_original_element();


10) /Users/lucilastancato/Documents/Tumblr/stream-builder/lib/Tumblr/StreamBuilder/Streams/Stream.php:72    [M] MethodCallRemoval

@@ @@
             $result = $this->_enumerate($count, $cursor, $tracer, $option);
             array_map(function (StreamElement $e) {
                 if ($e->getComponent() === null) {
-                    // do not override inner component tag
-                    $e->setComponent($this->getComponent());
+                    
                 }
                 return $e;
             }, $result->get_elements());



155 mutations were generated:
      25 mutants were killed
     120 mutants were configured to be ignored
       0 mutants were not covered by tests
      10 covered mutants were not detected
       0 errors were encountered
       0 syntax errors were encountered
       0 time outs were encountered
       0 mutants required more time than configured

Metrics:
         Mutation Score Indicator (MSI): 71%
         Mutation Code Coverage: 100%
         Covered Code MSI: 71%

Please note that some mutants will inevitably be harmless (i.e. false positives).

Time: 0s. Memory: 0.05GB. Threads: 16

@markbiek
Copy link
Contributor

markbiek commented Oct 24, 2025

@lucila I think the issue is that the Run Infection check is failing with this error:

The "--only-covered" option does not exist. 

Not sure if the infection version needs to be updated or if there's a way to change the arguments on that action?

@lucila
Copy link
Contributor Author

lucila commented Oct 24, 2025

@markbiek I have removed the option "--only-covered" from the makefile.

when I run the command locally, I get reported a few mutants. addressing these one file at a time

Copy link
Contributor

@markbiek markbiek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

I tested on WPCOM and things seem to be working as normal.

@lucila lucila requested a review from markbiek October 27, 2025 15:19
Copy link
Contributor

@markbiek markbiek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still LGTM!

@lucila lucila merged commit e3fc008 into main Oct 27, 2025
11 checks passed
@lucila lucila deleted the lucila/update-cappedpostranker branch October 27, 2025 15:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants