From 240fe0c87976170a992fb7a7c3f27726a5996923 Mon Sep 17 00:00:00 2001 From: James Mead Date: Mon, 17 Oct 2022 18:11:35 +0100 Subject: [PATCH 1/4] TEMP: Delete deployment-related GH actions We don't want to run those on this fork! --- .github/workflows/deploy.yaml | 45 ------------------- .../set-automatic-deploys-enabled.yaml | 37 --------------- 2 files changed, 82 deletions(-) delete mode 100644 .github/workflows/deploy.yaml delete mode 100644 .github/workflows/set-automatic-deploys-enabled.yaml diff --git a/.github/workflows/deploy.yaml b/.github/workflows/deploy.yaml deleted file mode 100644 index 82b088ccf0d..00000000000 --- a/.github/workflows/deploy.yaml +++ /dev/null @@ -1,45 +0,0 @@ -name: Deploy - -on: - workflow_dispatch: - inputs: - gitRef: - description: 'Commit, tag or branch name to deploy' - required: true - type: string - default: 'main' - environment: - description: 'Environment to deploy to' - required: true - type: choice - options: - - integration - - staging - - production - default: 'integration' - workflow_run: - workflows: [CI] - types: [completed] - branches: [main] - -jobs: - build-and-publish-image: - name: Build and publish image - if: github.event_name == 'workflow_dispatch' || github.event.workflow_run.conclusion == 'success' - uses: alphagov/govuk-infrastructure/.github/workflows/ci-ecr.yaml@main - with: - gitRef: ${{ github.event.inputs.gitRef || github.ref }} - secrets: - AWS_GOVUK_ECR_ACCESS_KEY_ID: ${{ secrets.AWS_GOVUK_ECR_ACCESS_KEY_ID }} - AWS_GOVUK_ECR_SECRET_ACCESS_KEY: ${{ secrets.AWS_GOVUK_ECR_SECRET_ACCESS_KEY }} - trigger-deploy: - name: Trigger deploy to ${{ github.event.inputs.environment }} - needs: build-and-publish-image - uses: alphagov/govuk-infrastructure/.github/workflows/deploy.yaml@main - with: - imageTag: ${{ needs.build-and-publish-image.outputs.imageTag }} - environment: ${{ github.event.inputs.environment }} - secrets: - WEBHOOK_TOKEN: ${{ secrets.GOVUK_INTEGRATION_ARGO_EVENTS_WEBHOOK_TOKEN }} - WEBHOOK_URL: ${{ secrets.GOVUK_INTEGRATION_ARGO_EVENTS_WEBHOOK_URL }} - GOVUK_CI_GITHUB_API_TOKEN: ${{ secrets.GOVUK_CI_GITHUB_API_TOKEN }} diff --git a/.github/workflows/set-automatic-deploys-enabled.yaml b/.github/workflows/set-automatic-deploys-enabled.yaml deleted file mode 100644 index 217b0ecba86..00000000000 --- a/.github/workflows/set-automatic-deploys-enabled.yaml +++ /dev/null @@ -1,37 +0,0 @@ -name: Set automatic_deploys_enabled (optionally image_tag too) - -on: - workflow_dispatch: - inputs: - resetImageTag: - description: 'Reset image tag to main' - required: false - default: false - type: boolean - automaticDeploysEnabled: - description: 'Activate automatic deploys' - required: false - default: true - type: boolean - environment: - description: 'Environment to deploy to' - required: true - type: choice - options: - - integration - - staging - - production - default: 'integration' - -jobs: - set_automatic_deploys_enabled: - name: Set automatic_deploys_enabled to ${{ github.event.inputs.environment }} - uses: alphagov/govuk-infrastructure/.github/workflows/set-automatic-deploys-enabled.yaml@main - with: - resetImageTag: ${{ github.event.inputs.resetImageTag == 'true' }} - automaticDeploysEnabled: ${{ github.event.inputs.automaticDeploysEnabled == 'true' }} - environment: ${{ github.event.inputs.environment }} - secrets: - WEBHOOK_TOKEN: ${{ secrets.GOVUK_INTEGRATION_ARGO_EVENTS_WEBHOOK_TOKEN }} - WEBHOOK_URL: ${{ secrets.GOVUK_INTEGRATION_ARGO_EVENTS_WEBHOOK_URL }} - GOVUK_CI_GITHUB_API_TOKEN: ${{ secrets.GOVUK_CI_GITHUB_API_TOKEN }} From f17ed2f0917f3427a655457e5468120840655208 Mon Sep 17 00:00:00 2001 From: James Mead Date: Mon, 17 Oct 2022 17:12:06 +0100 Subject: [PATCH 2/4] Upgrade Mocha to pre-release v2.0.0.alpha.1 --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile.lock b/Gemfile.lock index e6b645675c6..393ebdd6fe4 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -400,7 +400,7 @@ GEM minitest-stub-const (0.6) mlanett-redis-lock (0.2.7) redis - mocha (1.15.0) + mocha (2.0.0.alpha.1) msgpack (1.5.6) multi_json (1.15.0) multi_test (1.1.0) From 4b62bcbbd10080a597957a22095a8aae703239d1 Mon Sep 17 00:00:00 2001 From: James Mead Date: Tue, 18 Oct 2022 15:36:04 +0100 Subject: [PATCH 3/4] Avoid Mocha deprecation warning in test helper This avoids the following deprecation warning: Mocha deprecation warning at test/test_helper.rb:34:in `block in
': Configuration#reinstate_undocumented_behaviour_from_v1_9= is unnecessarily being set to false, because this is now the default value. Configuration#reinstate_undocumented_behaviour_from_v1_9= will be removed in the future, so you should avoid calling it. --- test/test_helper.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/test_helper.rb b/test/test_helper.rb index 6ad046e8377..6825976c2bb 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -31,7 +31,6 @@ Whitehall::Application.load_tasks if Rake::Task.tasks.empty? Mocha.configure do |c| - c.reinstate_undocumented_behaviour_from_v1_9 = false c.stubbing_non_existent_method = :prevent end From 0a434c0e2c0cdf3d909a4138c4d619dc038ef02b Mon Sep 17 00:00:00 2001 From: James Mead Date: Tue, 18 Oct 2022 17:28:39 +0100 Subject: [PATCH 4/4] WIP: Fix some Mocha deprecation warnings This fixes some of the warnings related to strict keyword argument matching, like this one: Mocha deprecation warning at app/services/asset_manager/attachment_updater/update.rb:15:in `call': Expectation defined at test/unit/services/asset_manager/attachment_updater/draft_status_updates_test.rb:28: in `block (3 levels) in ' expected keyword arguments ("draft" => true), but received positional hash ({"draft" => true}). These will stop matching when strict keyword argument matching is enabled. See the documentation for Mocha::Configuration#strict_keyword_argument_matching=. In order to fix the warnings, I've changed the signature of AssetManager::AssetUpdater#call so that new_attributes parameter must be supplied as keyword arguments rather than a positional Hash. This had the advantage that I only needed to change AssetManager::AttachmentUpdater::Update#call to convert the deep_stringify_keys into keyword arguments using a double-splat; I didn't need to change all the places where AssetManager::AssetUpdater#call is stubbed. An alternative would be to leave the signature of AssetManager::AssetUpdater#call unchanged and change the calls to Expectation#with for all the relevant stubs to pass a positional Hash, i.e. wrapped in braces. This article [1] is a very useful reference. [1]: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/ --- app/services/asset_manager/asset_updater.rb | 6 +++--- app/services/asset_manager/attachment_updater/update.rb | 2 +- test/unit/services/asset_manager/asset_updater_test.rb | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/services/asset_manager/asset_updater.rb b/app/services/asset_manager/asset_updater.rb index f1798d2769a..ff96dafd1fd 100644 --- a/app/services/asset_manager/asset_updater.rb +++ b/app/services/asset_manager/asset_updater.rb @@ -13,11 +13,11 @@ def initialize(attachment_data_id, legacy_url_path) end end - def self.call(*args) - new.call(*args) + def self.call(...) + new.call(...) end - def call(asset_data, legacy_url_path, new_attributes = {}) + def call(asset_data, legacy_url_path, **new_attributes) attributes = find_asset_by(legacy_url_path) asset_deleted = attributes["deleted"] diff --git a/app/services/asset_manager/attachment_updater/update.rb b/app/services/asset_manager/attachment_updater/update.rb index 2be39e4de56..941b2748981 100644 --- a/app/services/asset_manager/attachment_updater/update.rb +++ b/app/services/asset_manager/attachment_updater/update.rb @@ -15,7 +15,7 @@ def call AssetManager::AssetUpdater.call( attachment_data, legacy_url_path, - new_attributes.deep_stringify_keys, + **new_attributes.deep_stringify_keys, ) end diff --git a/test/unit/services/asset_manager/asset_updater_test.rb b/test/unit/services/asset_manager/asset_updater_test.rb index 6ebad92d7ec..b3c92a0d8f1 100644 --- a/test/unit/services/asset_manager/asset_updater_test.rb +++ b/test/unit/services/asset_manager/asset_updater_test.rb @@ -117,7 +117,7 @@ class AssetManager::AssetUpdaterTest < ActiveSupport::TestCase .with(@asset_id, "replacement_id" => replacement_id) attributes = { "replacement_legacy_url_path" => replacement_legacy_url_path } - @worker.call(@attachment_data, @legacy_url_path, attributes) + @worker.call(@attachment_data, @legacy_url_path, **attributes) end test "does not mark asset as replaced if already replaced by same asset" do @@ -130,6 +130,6 @@ class AssetManager::AssetUpdaterTest < ActiveSupport::TestCase Services.asset_manager.expects(:update_asset).never attributes = { "replacement_legacy_url_path" => replacement_legacy_url_path } - @worker.call(@attachment_data, @legacy_url_path, attributes) + @worker.call(@attachment_data, @legacy_url_path, **attributes) end end