Problem/Motivation

When a PHPStan job fails for core, a new baseline file artifact is generated. However, that is an incremental one (i.e. contains entries only for the NEW failures).

It would be more helpful to get a full baseline file so to copypaste it in the mr branch as is.

See https://drupal.slack.com/archives/CGKLP028K/p1730242123771039 for some insights.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#7 3484896-nr-bot.txt2.53 KBneeds-review-queue-bot

Issue fork drupal-3484896

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mondrake created an issue. See original summary.

mondrake’s picture

Issue summary: View changes

mondrake’s picture

Status: Active » Needs review
mondrake’s picture

After some headache, now found the right regex to have sed adjust the paths of the generated baseline so it can be really just copy/pasted to the branch with no additional edit.

bbrala’s picture

Status: Needs review » Reviewed & tested by the community

I actually ran into this issue earlier today by chance (the paths not being correct).

This looks good and will make some things easier, but also allow for a good interface in gitlab for changes as per #3499245: [CI] Report PHPStan baseline statistics in job.

All good, working as intended :)

needs-review-queue-bot’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new2.53 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

mondrake’s picture

Issue tags: +no-needs-review-bot

Please, bot, don't boycott.

mondrake’s picture

Status: Needs work » Reviewed & tested by the community
bbrala’s picture

Status: Reviewed & tested by the community » Needs work

As per Alex, we need a comment there, i understand it feels fragile, unfortunately a result of how things are run.

mondrake’s picture

Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Actually just reviewed a ticket that could of used this so perfect timing!

Believe feedback for a comment has been added so going to mark it.

longwave’s picture

Status: Reviewed & tested by the community » Needs work

I tried to commit this but it's breaking the pre-commit script:

Running PHPStan on changed files.
 ------ ----------------------------------------------------------------------- 
  Line                                                                          
 ------ ----------------------------------------------------------------------- 
  408    Syntax error, unexpected '"' on line 408                               
  408    Syntax error, unexpected T_STRING on line 408                          
  409    Syntax error, unexpected T_NS_SEPARATOR on line 409                    
  409    Syntax error, unexpected T_STRING on line 409                          
  426    Syntax error, unexpected T_LNUMBER on line 426                         
  450    Syntax error, unexpected T_STRING on line 450                          
  451    Syntax error, unexpected T_STRING on line 451                          
  479    Syntax error, unexpected T_STRING on line 479                          
  480    Syntax error, unexpected T_STRING on line 480                          
  514    Syntax error, unexpected '.' on line 514                               
  555    Syntax error, unexpected T_CONSTANT_ENCAPSED_STRING on line 555        
  555    Syntax error, unexpected T_STRING on line 555                          
  556    Syntax error, unexpected T_CONSTANT_ENCAPSED_STRING on line 556        
  556    Syntax error, unexpected T_STRING on line 556                          
  557    Syntax error, unexpected '"', expecting '-' or T_STRING or T_VARIABLE  
         or T_NUM_STRING on line 557                                            
  557    Syntax error, unexpected T_STRING, expecting T_VARIABLE or             
         T_ENCAPSED_AND_WHITESPACE or T_DOLLAR_OPEN_CURLY_BRACES or             
         T_CURLY_OPEN on line 557                                               

I think because line 406 has the PHP open tag:

        echo "<?php return [];" > ./core/.phpstan-baseline.php

then PHPStan tries to interpret the YAML as PHP and it all goes wrong. Either we need to tell PHPStan to ignore this file, or remove the open tag, or break it up into two parts so PHPStan can't see it?

mondrake’s picture

Why on earth is that script trying to have PHPStan scan .gitlab-ci.yml?

Is core/scripts/dev/commit-code-check.sh the culprit? If it is, then it's probably because if PHPStan is passed a list of file paths, it's skipping the file discovery based on its configuration and just scan them all. So that script need change to only pass files with php/module/inc/etc. extensions.

mondrake’s picture

Assigned: Unassigned » mondrake

OK, let's revert to prior https://git.drupalcode.org/project/drupal/-/merge_requests/10903/diffs?c... and see if that could fit.

mondrake’s picture

Assigned: mondrake » Unassigned
Status: Needs work » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

With all the recent rebasing people have to do around the baseline it would be nice to get this one in please :)

  • longwave committed 655596f2 on 11.x
    Issue #3484896 by mondrake, smustgrave, bbrala, longwave, alexpott: [CI...

longwave credited alexpott.

longwave’s picture

Status: Reviewed & tested by the community » Fixed

This is kinda messy, but I tried some alternatives that didn't work or were more messy, so I think this will have to do.

I am not sure we need all those $CI_PROJECT_DIRs as we are inconsistent there too, but let's just get this in and solve that elsewhere as we will surely make more changes here in the near future.

Committed 655596f and pushed to 11.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.