-
Notifications
You must be signed in to change notification settings - Fork 438
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.
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
Pre push testsuites #1136
base: v2.x
Are you sure you want to change the base?
Pre push testsuites #1136
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
#!/bin/sh | ||
|
||
$(ENV) | ||
|
||
# Run GrumPHP | ||
(cd "${HOOK_EXEC_PATH}" | $(EXEC_GRUMPHP_COMMAND) $(HOOK_COMMAND)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
#!/bin/sh | ||
|
||
$(ENV) | ||
|
||
# Run GrumPHP | ||
cd $(VAGRANT_HOST_DIR) && vagrant ssh --command '$(which sh)' << COMMANDS | ||
cd $(VAGRANT_PROJECT_DIR) | ||
|
||
# Add grumphp envs: | ||
$(ENV) | ||
|
||
$(EXEC_GRUMPHP_COMMAND) $(HOOK_COMMAND)) | ||
COMMANDS |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,115 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace GrumPHP\Console\Command\Git; | ||
|
||
use GrumPHP\Collection\FilesCollection; | ||
use GrumPHP\Collection\TestSuiteCollection; | ||
use GrumPHP\IO\IOFactory; | ||
use GrumPHP\IO\IOInterface; | ||
use GrumPHP\Locator\ChangedFiles; | ||
use GrumPHP\Locator\StdInFiles; | ||
use GrumPHP\Runner\TaskRunner; | ||
use GrumPHP\Runner\TaskRunnerContext; | ||
use GrumPHP\Task\Context\GitPrePushContext; | ||
use Symfony\Component\Console\Command\Command; | ||
use Symfony\Component\Console\Input\InputInterface; | ||
use Symfony\Component\Console\Input\InputOption; | ||
use Symfony\Component\Console\Output\OutputInterface; | ||
|
||
/** | ||
* This command runs the git pre-push hook. | ||
*/ | ||
class PrePushCommand extends Command | ||
{ | ||
const COMMAND_NAME = 'git:pre-push'; | ||
const EXIT_CODE_OK = 0; | ||
const EXIT_CODE_NOK = 1; | ||
|
||
/** | ||
* @var TestSuiteCollection | ||
*/ | ||
private $testSuites; | ||
|
||
/** | ||
* @var StdInFiles | ||
*/ | ||
private $stdInFilesLocator; | ||
|
||
/** | ||
* @var ChangedFiles | ||
*/ | ||
private $changedFilesLocator; | ||
|
||
/** | ||
* @var TaskRunner | ||
*/ | ||
private $taskRunner; | ||
|
||
private IOInterface $io; | ||
|
||
public function __construct( | ||
TestSuiteCollection $testSuites, | ||
StdInFiles $stdInFilesLocator, | ||
ChangedFiles $changedFilesLocator, | ||
TaskRunner $taskRunner, | ||
IOInterface $io | ||
) { | ||
parent::__construct(); | ||
|
||
$this->testSuites = $testSuites; | ||
$this->stdInFilesLocator = $stdInFilesLocator; | ||
$this->changedFilesLocator = $changedFilesLocator; | ||
$this->taskRunner = $taskRunner; | ||
$this->io = $io; | ||
} | ||
|
||
public static function getDefaultName(): string | ||
{ | ||
return self::COMMAND_NAME; | ||
} | ||
|
||
protected function configure(): void | ||
{ | ||
$this->setDescription('Executed by the pre-push hook'); | ||
$this->addOption( | ||
'skip-success-output', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should output be ever skipped? |
||
null, | ||
InputOption::VALUE_NONE, | ||
'Skips the success output. This will be shown by another command in the git commit hook chain.' | ||
); | ||
} | ||
|
||
public function execute(InputInterface $input, OutputInterface $output): int | ||
{ | ||
$localRef = $this->changedFilesLocator->getLocalRef(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we need to introduce a new PushedFiles locator which contains all of this logic and the I'm not sure how it behaves exactly, but I'm sure test-cases will be added to show how it works exactly. |
||
$localSha = $this->changedFilesLocator->getLocalSHA1(); | ||
$remoteSha = $this->changedFilesLocator->getRemoteSHA1(); | ||
|
||
$taskRunnerContext = ( | ||
new TaskRunnerContext( | ||
new GitPrePushContext($this->getPushedFiles($localRef, $localSha, $remoteSha)), | ||
$this->testSuites->getOptional('git_pre_push') | ||
) | ||
)->withSkippedSuccessOutput((bool) $input->getOption('skip-success-output')); | ||
|
||
$output->writeln('<fg=yellow>GrumPHP detected a pre-push command.</fg=yellow>'); | ||
$results = $this->taskRunner->run($taskRunnerContext); | ||
|
||
return $results->isFailed() ? self::EXIT_CODE_NOK : self::EXIT_CODE_OK; | ||
} | ||
|
||
protected function getPushedFiles(?string $localRef, ?string $localSha, ?string $remoteSha): FilesCollection | ||
{ | ||
if (empty($remoteSha)) { | ||
return $this->changedFilesLocator->locateFromRawDiffInput('HEAD'); | ||
} | ||
|
||
if ($localRef === '(delete)') { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should the task run if you delete a branch ? |
||
return new FilesCollection([]); | ||
} | ||
|
||
return $this->changedFilesLocator->locateFromGitDiff($remoteSha, $localSha); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,7 +10,7 @@ | |
use GrumPHP\Git\GitRepository; | ||
use GrumPHP\Util\Filesystem; | ||
use GrumPHP\Util\Paths; | ||
use Symfony\Component\Finder\SplFileInfo; | ||
use SplFileInfo; | ||
|
||
/** | ||
* Class Git. | ||
|
@@ -46,6 +46,66 @@ public function locateFromGitRepository(): FilesCollection | |
return $this->parseFilesFromDiff($diff); | ||
} | ||
|
||
public function getLocalRef(): ?string | ||
{ | ||
return $this->repository->run('symbolic-ref', ['HEAD']); | ||
} | ||
public function getLocalSHA1(): ?string | ||
{ | ||
return $this->repository->run('rev-parse', ['HEAD']); | ||
} | ||
private function getRemoteRef(): ?string | ||
{ | ||
return $this->repository->tryToRunWithFallback( | ||
function (): ?string { | ||
return $this->repository->run('rev-parse', ['--abbrev-ref', '--symbolic-full-name', '@{u}']); | ||
}, | ||
'null' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. having a |
||
); | ||
} | ||
|
||
public function getRemoteSHA1(): ?string | ||
{ | ||
if ($this->getRemoteRef() === 'null') { | ||
return null; | ||
} | ||
/** | ||
* @psalm-suppress PossiblyUndefinedArrayOffset | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe better to validate the format and return null if it's unexpected? |
||
* @psalm-suppress PossiblyNullArgument | ||
*/ | ||
list($remoteName, $branchName) = explode('/', $this->getRemoteRef(), 2); | ||
$output = $this->repository->run('ls-remote', [$remoteName, $branchName]); | ||
return $output ? strtok($output, "\t") : null; | ||
} | ||
|
||
/** | ||
* @return FilesCollection | ||
*/ | ||
public function locateFromGitDiff(string $fromSha, ?string $toSha): FilesCollection | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What worries me a bit, is that there seem to be a lot of different states possible which require slightly different approaches / solutions. Maybe the new PushedFiled locator should also contain details on how it works and what it covers exactly. |
||
{ | ||
if (empty($toSha)) { | ||
$toSha = 'HEAD'; | ||
} | ||
$files = []; | ||
$commits = $fromSha . '..' . $toSha; | ||
$diff = $this->repository->tryToRunWithFallback( | ||
function () use ($commits): ?string { | ||
return $this->repository->run('diff', ['--name-only', trim($commits), '--oneline']); | ||
}, | ||
'null' | ||
); | ||
if ($diff === 'null') { | ||
return new FilesCollection($files); | ||
} | ||
|
||
foreach (explode("\n", $diff) as $file) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would this always be |
||
$fileObject = new SplFileInfo($file); | ||
$files[] = $fileObject; | ||
} | ||
|
||
return new FilesCollection($files); | ||
} | ||
|
||
public function locateFromRawDiffInput(string $rawDiff): FilesCollection | ||
{ | ||
$diff = $this->repository->createRawDiff($rawDiff); | ||
|
@@ -75,6 +135,6 @@ private function makeFileRelativeToProjectDir(File $file): SplFileInfo | |
$file->isRename() ? $file->getNewName() : $file->getName() | ||
); | ||
|
||
return new SplFileInfo($filePath, dirname($filePath), $filePath); | ||
return new SplFileInfo($filePath); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any specific reason why this was changed? |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
use GrumPHP\Task\Config\ConfigOptionsResolver; | ||
use GrumPHP\Task\Context\ContextInterface; | ||
use GrumPHP\Task\Context\GitPreCommitContext; | ||
use GrumPHP\Task\Context\GitPrePushContext; | ||
use GrumPHP\Task\Context\RunContext; | ||
use Symfony\Component\OptionsResolver\OptionsResolver; | ||
|
||
|
@@ -36,7 +37,9 @@ public static function getConfigurableOptions(): ConfigOptionsResolver | |
|
||
public function canRunInContext(ContextInterface $context): bool | ||
{ | ||
return $context instanceof GitPreCommitContext || $context instanceof RunContext; | ||
return $context instanceof GitPreCommitContext | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By default, this will run the task in both pre-commit and pre-push scenario? Would it make sense to have a new metadata field that can be used to change the context instead? Something like: grumphp:
tasks:
anytask:
metadata:
git_stage: ["pre-commit", "pre-push"] Which can be used to specify at what stages in your git flow you want to run this task. WDYT? |
||
|| $context instanceof GitPrePushContext | ||
|| $context instanceof RunContext; | ||
} | ||
|
||
public function run(ContextInterface $context): TaskResultInterface | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The STDIN files locator is not being used