bugfix: fix finding commits since release

composer.json

@@ -21,6 +21,7 @@
         "kriswallsmith/buzz": "^1.2",
         "nyholm/psr7": "^1.4",
         "php-http/discovery": "^1.19",
+        "symfony/console": "^6.0|^7.0|^8.0",
         "symfony/polyfill-php82": "^1.26",
         "symfony/polyfill-php83": "^1.32",
         "symfony/polyfill-php84": "^1.32",
@@ -41,9 +42,9 @@
     "require-dev": {
         "bamarni/composer-bin-plugin": "^1.8",
         "mikey179/vfsstream": "^1.6",
-        "phpstan/phpstan": "^1.10",
-        "phpstan/phpstan-mockery": "^1.0",
-        "phpstan/phpstan-phpunit": "^1.0",
+        "phpstan/phpstan": "^2.1",
+        "phpstan/phpstan-mockery": "^2.0",
+        "phpstan/phpstan-phpunit": "^2.0",
         "phpunit/phpunit": "^9.3"
     },
     "scripts": {

src/Console/Command/Release/AbstractReleaseCommand.php

@@ -92,7 +92,8 @@ protected function get_latest_release(Repository $repository): ?Release
         $data = json_decode($response->getBody()->getContents());
 
         $release = new Release();
-        $release->timestamp = $data->published_at;
+        //created_at is when the tag was created, which may be well before the release was published
+        $release->timestamp = $data->created_at;
         $release->version = $data->tag_name;
         $this->output->isVerbose() && $this->output->writeln("[INFO] Latest release of {$repository->downstream} is {$release}");
 
@@ -102,22 +103,37 @@ protected function get_latest_release(Repository $repository): ?Release
     protected function get_downstream_unreleased_commits(Repository $repository): array
     {
         $commits_url = "https://api.github.com/repos/{$repository->downstream}/commits";
-        if ($repository->latestRelease !== null) {
-            $commits_url .= "?since={$repository->latestRelease->timestamp}";
+        $latestRelease = $repository->latestRelease;
+        if ($latestRelease !== null) {
+            $commits_url .= '?since=' . $this->since($latestRelease);
         }
 
         return $this->get_commits($commits_url, $repository);
     }
 
+    /**
+     * Github's `since` filter is inclusive, so advance one second past the tag to avoid listing the
+     * already-released commit. This happens with lightweight tags, where the tag's timestamp is the
+     * timestamp of the commit it points at.
+     */
+    private function since(Release $release): string
+    {
+        return (new \DateTimeImmutable($release->timestamp))
+            ->setTimezone(new \DateTimeZone('UTC'))
+            ->modify('+1 second')
+            ->format('Y-m-d\TH:i:s\Z');
+    }
+
     /**
      * @param Repository $repository
      * @return array<Commit>
      */
     protected function get_upstream_unreleased_commits(Repository $repository): array
     {
         $commits_url = "https://api.github.com/repos/{$repository->upstream}/commits?path={$repository->upstream->path}";
-        if ($repository->latestRelease !== null) {
-            $commits_url .= "&since={$repository->latestRelease->timestamp}";
+        $latestRelease = $repository->latestRelease;
+        if ($latestRelease !== null) {
+            $commits_url .= '&since=' . $this->since($latestRelease);
         }
 
         return $this->get_commits($commits_url, $repository);

src/Console/Release/Repository.php

@@ -8,7 +8,7 @@ class Repository
 {
     public Project $upstream; //the monorepo that this repo is split from
     public Project $downstream; //the read-only downstream repo (split target)
-    public ?Release $latestRelease; //latest release
+    public ?Release $latestRelease = null; //latest release
     /**
      * @var array<Commit>
      */

tests/Unit/Package/Composer/TestInstallationFactoryTest.php

@@ -10,6 +10,7 @@
 use OpenTelemetry\DevTools\Package\Composer\TestInstallationFactory;
 use OpenTelemetry\DevTools\Package\Composer\ValueObject\RepositoryCollection;
 use OpenTelemetry\DevTools\Package\Composer\ValueObject\SingleRepositoryInterface;
+use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 
 /**
@@ -21,7 +22,7 @@ class TestInstallationFactoryTest extends TestCase
     private const TESTED_BRANCH_VERSION = 'dev-' . self::TESTED_BRANCH;
 
     private TestInstallationFactory $instance;
-    private TestConfigFactory $testConfigFactory;
+    private TestConfigFactory&MockObject $testConfigFactory;
 
     #[\Override]
     protected function setUp(): void
@@ -33,13 +34,9 @@ protected function setUp(): void
         );
     }
 
-    /**
-     * @psalm-suppress UndefinedMethod
-     */
     public function test_build(): void
     {
         $config = $this->createMock(TestConfig::class);
-        /** @phpstan-ignore-next-line */
         $this->testConfigFactory
             ->method('build')
             ->willReturn($config);
@@ -85,12 +82,8 @@ public function test_get_config_factory(): void
         );
     }
 
-    /**
-     * @psalm-suppress UndefinedMethod
-     */
     public function test_add_default_dependency(): void
     {
-        /** @phpstan-ignore-next-line */
         $this->testConfigFactory
             ->expects($this->once())
             ->method('addDefaultDependency');

tests/Unit/Package/Composer/TestInstallationTest.php

@@ -14,6 +14,7 @@
 use OpenTelemetry\DevTools\Package\Composer\ValueObject\SingleRepositoryInterface;
 use org\bovigo\vfs\vfsStream;
 use org\bovigo\vfs\vfsStreamDirectory;
+use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use RuntimeException;
 
@@ -35,9 +36,9 @@ class TestInstallationTest extends TestCase
     ];
 
     private TestInstallation $instance;
-    private SingleRepositoryInterface $repository;
-    private TestConfig $config;
-    private RepositoryCollection $dependencies;
+    private SingleRepositoryInterface&MockObject $repository;
+    private TestConfig&MockObject $config;
+    private RepositoryCollection&MockObject $dependencies;
     private vfsStreamDirectory $root;
     private string $testDirectory;
 
@@ -106,11 +107,9 @@ public function test_get_tested_branch(): void
 
     /**
      * @throws JsonException
-     * @psalm-suppress UndefinedMethod
      */
     public function test_to_json(): void
     {
-        /** @phpstan-ignore-next-line */
         $this->config
             ->method('toArray')
             ->willReturn(self::TEST_CONFIG);
@@ -121,12 +120,8 @@ public function test_to_json(): void
         );
     }
 
-    /**
-     * @psalm-suppress UndefinedMethod
-     */
     public function test_to_json_throws_exception_on_invalid_json(): void
     {
-        /** @phpstan-ignore-next-line */
         $this->config
             ->method('toArray')
             ->willThrowException(
@@ -140,11 +135,9 @@ public function test_to_json_throws_exception_on_invalid_json(): void
 
     /**
      * @throws JsonException
-     * @psalm-suppress UndefinedMethod
      */
     public function test_to_string(): void
     {
-        /** @phpstan-ignore-next-line */
         $this->config
             ->method('toArray')
             ->willReturn(self::TEST_CONFIG);
@@ -155,18 +148,13 @@ public function test_to_string(): void
         );
     }
 
-    /**
-     * @psalm-suppress UndefinedMethod
-     */
     public function test_write_composer_file(): void
     {
         $composerPath = $this->testDirectory . DIRECTORY_SEPARATOR . TestInstallation::COMPOSER_FILE_NAME;
-        /** @phpstan-ignore-next-line */
         $this->repository
             ->method('getComposerFilePath')
             ->willReturn($composerPath);
 
-        /** @phpstan-ignore-next-line */
         $this->config
             ->method('toArray')
             ->willReturn(self::TEST_CONFIG);
@@ -181,19 +169,14 @@ public function test_write_composer_file(): void
         );
     }
 
-    /**
-     * @psalm-suppress UndefinedMethod
-     */
     public function test_write_composer_file_throws_exception_on_file_write_error(): void
     {
         $composerPath = $this->testDirectory . DIRECTORY_SEPARATOR . TestInstallation::COMPOSER_FILE_NAME;
-        /** @phpstan-ignore-next-line */
         $this->repository->method('getComposerFilePath')
             ->willReturn('foo://bar.baz');
 
         $this->expectException(RuntimeException::class);
 
-        /** @phpstan-ignore-next-line */
         $this->config
             ->method('toArray')
             ->willReturn(self::TEST_CONFIG);

tests/Unit/Package/Composer/TestInstallerTest.php

@@ -11,6 +11,7 @@
 use OpenTelemetry\DevTools\Util\RecursiveDirectoryRemover;
 use org\bovigo\vfs\vfsStream;
 use org\bovigo\vfs\vfsStreamDirectory;
+use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use RuntimeException;
 
@@ -24,7 +25,7 @@ class TestInstallerTest extends TestCase
     public const COMPOSER_FILE_NAME = 'composer.json';
 
     private TestInstaller $instance;
-    private RecursiveDirectoryRemover $directoryRemover;
+    private RecursiveDirectoryRemover&MockObject $directoryRemover;
     private vfsStreamDirectory $root;
 
     #[\Override]
@@ -46,17 +47,13 @@ protected function tearDown(): void
         TestInstaller::setDirectoryRemover(null);
     }
 
-    /**
-     * @psalm-suppress UndefinedMethod
-     */
     public function test_install(): void
     {
         $composerDirectory = self::TEST_DIR;
         $testDirectory = vfsStream::newDirectory(self::TEST_DIR, 0777)
             ->at($this->root)
             ->url();
 
-        /** @phpstan-ignore-next-line */
         $this->directoryRemover->method('remove')
             ->willReturnCallback(function () use ($testDirectory) {
                 rmdir($testDirectory);
@@ -92,9 +89,6 @@ public function test_install_throws_exception_on_error(): void
         );
     }
 
-    /**
-     * @psalm-suppress UndefinedMethod
-     */
     public function test_install_throws_exception_when_not_able_to_create_install_directory(): void
     {
         $this->expectException(InvalidArgumentException::class);
@@ -104,7 +98,6 @@ public function test_install_throws_exception_when_not_able_to_create_install_di
         $this->root->chown(12345);
         $this->root->chmod(0700);
 
-        /** @phpstan-ignore-next-line */
         $this->directoryRemover->method('remove')
             ->willReturn(true);
 
@@ -136,9 +129,6 @@ public function test_remove(): void
         );
     }
 
-    /**
-     * @psalm-suppress UndefinedMethod
-     */
     public function test_remove_throws_exception_on_error(): void
     {
         $exception = new RuntimeException();
@@ -147,7 +137,6 @@ public function test_remove_throws_exception_on_error(): void
 
         $installation = $this->createMock(TestInstallation::class);
 
-        /** @phpstan-ignore-next-line */
         $this->directoryRemover->method('remove')
             ->willThrowException($exception);
 

tests/Unit/Package/Composer/ValueObject/AbstractCollectionTest.php

@@ -43,7 +43,7 @@ public function test_offset_exists(): void
         $this->instance[self::OFFSET_KEY] = $this->createMock(ValueObjectInterface::class);
 
         $this->assertTrue(
-            isset($this->instance[self::OFFSET_KEY])
+            $this->instance->offsetExists(self::OFFSET_KEY)
         );
     }