Skip to content
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

FIX Remove duplicate jobs #111

Merged
merged 1 commit into from
Oct 15, 2024
Merged

FIX Remove duplicate jobs #111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions job_creator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,10 @@ public function createJson(string $yml): string
$matrix['include'][$i]['name'] = implode(' ', $name);
}

// ensure there are no duplicate jobs
$uniqueSerializedJobs = array_unique(array_map('serialize', $matrix['include']));
$matrix['include'] = array_values(array_map('unserialize', $uniqueSerializedJobs));

// output json, keep it on a single line so do not use pretty print
$json = json_encode($matrix, JSON_UNESCAPED_SLASHES);
// Remove indents
Expand Down
42 changes: 42 additions & 0 deletions tests/JobCreatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1949,4 +1949,46 @@ public function provideComposerInstall(): array
],
];
}

public function testDuplicateJobsRemoved(): void
{
if (!function_exists('yaml_parse')) {
$this->markTestSkipped('yaml extension is not installed');
}
Comment on lines +1955 to +1957
Copy link
Member

@GuySartorelli GuySartorelli Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We really shouldn't skip these - it should just be a requirement that php yaml is installed if you wanna run tests. OTherwise it looks like everything's fine when stuff is just being skipped but might be broken.

There's more than just this one test doing it though so that's a discussion to be held outside this PR.

$yml = implode("\n", [
$this->getGenericYml(),
<<<EOT
github_repository: 'myaccount/silverstripe-framework'
github_my_ref: '5'
parent_branch: ''
extra_jobs:
- php: 8.2
phpunit: true
phpunit_suite: fish
- php: 8.3
phpunit: true
phpunit_suite: fish
- php: 8.3
phpunit: true
phpunit_suite: fish
- php: 8.3
endtoend: true
GuySartorelli marked this conversation as resolved.
Show resolved Hide resolved
EOT
]);
$creator = new JobCreator();
$json = json_decode($creator->createJson($yml));
$actual = [];
foreach ($json->include as $job) {
$actual[] = $job->name;
}
$expected = [
'8.1 prf-low mysql57 phpunit all',
'8.2 mariadb phpunit all',
'8.3 mysql80 phpunit all',
'8.2 mysql57 phpunit fish',
'8.3 mysql57 phpunit fish',
'8.3 mysql57 endtoend root',
];
$this->assertSame($expected, $actual);
}
}