Skip to content

Fix label spacing in PR titles #275

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Aug 9, 2025
Merged

Fix label spacing in PR titles #275

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
9c7dac8
Add comprehensive test cases for label spacing in PR titles
OskarStark Aug 8, 2025
afea77d
Remove redundant testIncrementalLabelAdditions test
OskarStark Aug 8, 2025
08b3f76
Fix label spacing when title contains unrecognized bracketed text
OskarStark Aug 8, 2025
183f3bb
Update src/Subscriber/AutoUpdateTitleWithLabelSubscriber.php
OskarStark Aug 8, 2025
67b5162
Update tests/Subscriber/AutoUpdateTitleWithLabelSubscriberTest.php
OskarStark Aug 9, 2025
f06e5c5
Update AutoUpdateTitleWithLabelSubscriberTest.php
OskarStark Aug 9, 2025
eb21d52
Fix code style issues
OskarStark Aug 8, 2025
f0f67f5
Remove explanatory comment from test
OskarStark Aug 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/Subscriber/AutoUpdateTitleWithLabelSubscriber.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,22 @@ public function onPullRequest(GitHubEvent $event): void
// Clean string from all HTML chars and remove whitespace at the beginning
$prTitle = (string) preg_replace('@^[\h\s]+@u', '', html_entity_decode($prTitle));

// Add back labels
$prTitle = trim($prPrefix.' '.trim($prTitle));
// Extract any bracketed text at the beginning of the title
$leadingBrackets = '';
$remainingTitle = $prTitle;

// Match all consecutive bracketed items at the start of the title
while (preg_match('/^\[([^]]+)]\s*/', $remainingTitle, $matches)) {
$leadingBrackets .= '['.$matches[1].']';
$remainingTitle = substr($remainingTitle, strlen($matches[0]));
}

// Combine: valid labels + any unrecognized brackets + remaining title
if ('' !== trim($remainingTitle)) {
$prTitle = $prPrefix.$leadingBrackets.' '.trim($remainingTitle);
} else {
$prTitle = $prPrefix.$leadingBrackets;
}
if ($originalTitle === $prTitle) {
return;
}
Expand Down
169 changes: 167 additions & 2 deletions tests/Subscriber/AutoUpdateTitleWithLabelSubscriberTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public function testOnPullRequestLabeled()

$this->assertCount(2, $responseData);
$this->assertSame(1234, $responseData['pull_request']);
$this->assertSame('[Console][FrameworkBundle] [bar] Foo', $responseData['new_title']);
$this->assertSame('[Console][FrameworkBundle][bar] Foo', $responseData['new_title']);
}

public function testOnPullRequestLabeledCaseInsensitive()
Expand Down Expand Up @@ -110,7 +110,7 @@ public function testRemoveLabel()
$responseData = $event->getResponseData();
$this->assertCount(2, $responseData);
$this->assertSame(1234, $responseData['pull_request']);
$this->assertSame('[Console] [Random] Foo normal title', $responseData['new_title']);
$this->assertSame('[Console][Random] Foo normal title', $responseData['new_title']);
}

public function testExtraBlankSpace()
Expand All @@ -129,4 +129,169 @@ public function testExtraBlankSpace()
$this->assertSame(57753, $responseData['pull_request']);
$this->assertSame('[ErrorHandler] restrict the maximum length of the X-Debug-Exception header', $responseData['new_title']);
}

public function testMultipleLabelsWithoutSpaceBetweenBrackets()
{
$event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
$this->pullRequestApi->method('show')->willReturn([
'title' => 'Foo Bar',
'labels' => [
['name' => 'Platform', 'color' => 'dddddd'],
['name' => 'Agent', 'color' => 'dddddd'],
],
]);

$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
$responseData = $event->getResponseData();
$this->assertCount(2, $responseData);
$this->assertSame(1234, $responseData['pull_request']);
$this->assertSame('[Agent][Platform] Foo Bar', $responseData['new_title']);
}

public function testMultipleLabelsUpdateRemovesSpaceBetweenBrackets()
{
$event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
$this->pullRequestApi->method('show')->willReturn([
'title' => '[Platform] [Agent] Foo Bar', // Title already has labels with space
'labels' => [
['name' => 'Platform', 'color' => 'dddddd'],
['name' => 'Agent', 'color' => 'dddddd'],
],
]);

$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
$responseData = $event->getResponseData();
$this->assertCount(2, $responseData);
$this->assertSame(1234, $responseData['pull_request']);
$this->assertSame('[Agent][Platform] Foo Bar', $responseData['new_title']);
}

public function testAddingLabelToExistingLabeledTitle()
{
// Simulating when we already have [Platform] and are adding Agent label
$event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
$this->pullRequestApi->method('show')->willReturn([
'title' => '[Platform] Foo Bar', // Title already has one label
'labels' => [
['name' => 'Platform', 'color' => 'dddddd'],
['name' => 'Agent', 'color' => 'dddddd'], // New label added
],
]);

$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
$responseData = $event->getResponseData();
$this->assertCount(2, $responseData);
$this->assertSame(1234, $responseData['pull_request']);
$this->assertSame('[Agent][Platform] Foo Bar', $responseData['new_title']);
}

public function testLabelsNotRecognizedAsValidLabels()
{
// This test simulates what happens when [Platform] and [Agent] are in the title
// but they're NOT recognized as valid labels (wrong case or not in label list)
$event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
$this->pullRequestApi->method('show')->willReturn([
'title' => '[Platform] [Agent] Foo Bar', // These are in title but not recognized
'labels' => [
['name' => 'Bug', 'color' => 'dddddd'], // Different valid label
],
]);

$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
$responseData = $event->getResponseData();
$this->assertCount(2, $responseData);
$this->assertSame(1234, $responseData['pull_request']);
// Since Platform and Agent are not recognized labels, they stay in the title without spaces
$this->assertSame('[Bug][Platform][Agent] Foo Bar', $responseData['new_title']);
}

public function testBugWithLabelsNotInRepositoryLabelList()
{
$event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
$this->pullRequestApi->method('show')->willReturn([
'title' => 'Foo Bar',
'labels' => [
// These labels are attached to PR but not in the StaticLabelApi list
['name' => 'Platform', 'color' => 'dddddd'],
['name' => 'Agent', 'color' => 'dddddd'],
],
]);

$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
$responseData = $event->getResponseData();

$this->assertCount(2, $responseData);
$this->assertSame(1234, $responseData['pull_request']);
$this->assertSame('[Agent][Platform] Foo Bar', $responseData['new_title']);
}

/**
* Test that ensures no spaces are ever added between label brackets.
*/
public function testNoSpacesBetweenLabelBrackets()
{
// Test with empty title after label removal
$event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
$this->pullRequestApi->method('show')->willReturn([
'title' => '[Console] [FrameworkBundle]', // Only labels, no other text
'labels' => [
['name' => 'Console', 'color' => 'dddddd'],
['name' => 'FrameworkBundle', 'color' => 'dddddd'],
],
]);

$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
$responseData = $event->getResponseData();

// Should produce labels without spaces and no trailing space
$this->assertCount(2, $responseData);
$this->assertSame(1234, $responseData['pull_request']);
$this->assertSame('[Console][FrameworkBundle]', $responseData['new_title']);
}

/**
* Test when title has unrecognized bracketed text like [Foo] that isn't a label.
*/
public function testUnrecognizedBracketedTextWithNewLabel()
{
$event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
$this->pullRequestApi->method('show')->willReturn([
'title' => '[Foo] Bar', // [Foo] is not a recognized label
'labels' => [
['name' => 'Console', 'color' => 'dddddd'], // Adding Console label
],
]);

$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
$responseData = $event->getResponseData();

// Currently produces: [Console] [Foo] Bar (with space)
// Should produce: [Console][Foo] Bar (no space between brackets)
$this->assertCount(2, $responseData);
$this->assertSame(1234, $responseData['pull_request']);
$this->assertSame('[Console][Foo] Bar', $responseData['new_title']);
}

/**
* Test multiple unrecognized bracketed texts with real labels.
*/
public function testMultipleUnrecognizedBracketsWithRealLabels()
{
$event = new GitHubEvent(['action' => 'labeled', 'number' => 1234, 'pull_request' => []], $this->repository);
$this->pullRequestApi->method('show')->willReturn([
'title' => '[TODO] [WIP] [Foo] Some title', // [TODO], [WIP], [Foo] are not recognized labels
'labels' => [
['name' => 'Console', 'color' => 'dddddd'],
['name' => 'FrameworkBundle', 'color' => 'dddddd'],
],
]);

$this->dispatcher->dispatch($event, GitHubEvents::PULL_REQUEST);
$responseData = $event->getResponseData();

// Should keep unrecognized brackets but without spaces between any brackets
$this->assertCount(2, $responseData);
$this->assertSame(1234, $responseData['pull_request']);
$this->assertSame('[Console][FrameworkBundle][TODO][WIP][Foo] Some title', $responseData['new_title']);
}
}