CLI-1389: push.artifact.destination_git_urls in acquia cli config

[joshirohit100]

Fix #1788

Fix CLI-1389

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 92.23%. Comparing base (04b8ee7) to head (11c8a50). Report is 1 commits behind head on main.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #1789 +/- ## ============================================ + Coverage 92.22% 92.23% +0.01% Complexity 1817 1817 ============================================ Files 121 121 Lines 6813 6823 +10 ============================================ + Hits 6283 6293 +10 Misses 530 530 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[github-actions[bot]]

Try the dev build for this PR: https://acquia-cli.s3.amazonaws.com/build/pr/1789/acli.phar

curl -OL https://acquia-cli.s3.amazonaws.com/build/pr/1789/acli.phar
chmod +x acli.phar

[danepowell]

It looks like datastore support for Git URLs was added in https://github.com/acquia/cli/pull/733 but we weren't doing schema validation of the yaml store at that time. When we added the config tree validator I'm not sure why tests kept passing 🤷

[danepowell]

This looks good but we need to figure out why tests didn't catch this to prevent it from regressing again.

Let me know if that's something you can figure out, or we can try to get this into an upcoming sprint.

[joshirohit100]

I think command failing and test not failing because -

For command when runs, container builds and thus acsf command factory or api command factory loads and loads other services - https://github.com/acquia/cli/blob/main/src/Command/Acsf/AcsfCommandFactory.php#L41 which loads the dataStore service. this further loads yamlstore service and that calls https://github.com/acquia/cli/blob/main/src/DataStore/Datastore.php#L69 and there this processConfiguration internally calls the treebuilder to validate the tree

and given in project repo, this acli config file already exists and thus validation fails while building the tree on container load / initialize

In test run, this acli data store service is created from https://github.com/acquia/cli/blob/main/tests/phpunit/src/TestBase.php#L820 and as there is no acli config file exists, this passes and then there is no schema validation is done when something is set - https://github.com/acquia/cli/blob/main/tests/phpunit/src/Commands/Push/PushArtifactCommandTest.php#L128

SO in https://github.com/acquia/cli/blob/main/tests/phpunit/src/TestBase.php#L75, I changed this to

protected array $acliConfig = [
        'cloud_app_uuid' => 'some-app-id',
        'push' => [
            'artifact' => [
                'destination-git-urls' => [
                    'aa',
                    'bb',
                ]
            ],
        ],
    ];

and test failed . This is called from https://github.com/acquia/cli/blob/main/tests/phpunit/src/TestBase.php#L428

PHPUnit 9.6.20 by Sebastian Bergmann and contributors.

Test 'Acquia\Cli\Tests\Commands\Push\PushArtifactCommandTest::testPushArtifactWithAcquiaCliFile' started
Test 'Acquia\Cli\Tests\Commands\Push\PushArtifactCommandTest::testPushArtifactWithAcquiaCliFile' ended

Time: 00:00.032, Memory: 28.00 MB

There was 1 error:

1) Acquia\Cli\Tests\Commands\Push\PushArtifactCommandTest::testPushArtifactWithAcquiaCliFile
Acquia\Cli\Exception\AcquiaCliException: Configuration file at the following path contains invalid keys: vfs://root/project/.acquia-cli.yml Unrecognized option "push" under "acquia_cli". Available option is "cloud_app_uuid".

[danepowell]

@joshirohit100 thanks a lot for diving into that, I think you're right, the problem is we used acliDatastore->set() to establish test fixtures and this bypassed validation. We should have written the datastores as files and then reloaded the datstore/command to trigger validation. I'll fix that in #1796