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.

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

move most of integtest into a callable workflow #45

Merged
3 commits merged into from
Jan 9, 2023
Merged

Conversation

ghost
Copy link

@ghost ghost commented Jan 9, 2023

Rather than integtest.yaml containing all of the integ test logic, it now just invokes a new workflow, which contains all of its old logic. This makes it possible to invoke the tests from other repos.

It seems silly to create a workflow that does nothing but call another workflow, but I think that's necessary because during a cron-triggered run, the inputs.* object is empty (not populated with the default values).

It's also silly and a bit brittle to have the workflow_dispatch and workflow_call sections duplicate the same inputs, but unfortunately that seems to be what GH Actions require.

As part of this, rename integtest.yaml to nightly-integ-tests.yaml. This will kill the existing workflow history, but there's not much of it right now anyway.

Standard checks

  • Unit tests: n/a
  • Docs: none needed
  • Backwards compatibility: will reset the GH action history (link), but otherwise no issues

Yuval Shavit added 2 commits January 9, 2023 11:29
Rather than integtest.yaml containing all of the integ test logic, it
now just invokes a new workflow, which contains all of its old logic.
This makes it possible to invoke the tests from other repos.

It seems silly to create a workflow that does nothing but call another
workflow, but I _think_ that's necessary because during a cron-triggered
run, the `inputs.*` object is empty (not populated with the default
values).

It's also silly and a bit brittle to have the workflow_dispatch and
workflow_call sections duplicate the same inputs, but unfortunately that
seems to be what GH Actions require.

As part of this, rename integtest.yaml to nightly-integ-tests.yaml. This
will kill the existing workflow history, but there's not much of it
right now anyway.
See klothoplatform/klotho-history#259
Copy link
Contributor

@gordon-klotho gordon-klotho left a comment

Choose a reason for hiding this comment

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

I'm not clear what this change enables us to do - can you describe a little more?

@@ -84,14 +125,20 @@ jobs:
app_to_test: ${{ fromJson(needs.list-apps.outputs.to_test) }}
mode: [fresh, upgrade]
exclude:
- # issue $618
- # issue #618
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: These reference tickets on the old repo.

Copy link
Author

Choose a reason for hiding this comment

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

yeah, I wasn't really sure what to do with these in general. Should I refer to it as klotho-history#618? Do we want to advertise that in-the-code-ly the existence of that repo?

We can't move issues from a private repo to a public one (I tried), or else I'd have just moved it over as part of this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably have to just move them the old fashioned way - good 'ol C-c C-v 😄

@@ -188,7 +240,7 @@ jobs:
PULUMI_CONFIG_PASSPHRASE: ""
- name: pulumi up (upgrade path)
if: matrix.mode == 'upgrade'
uses: klothoplatform/gh-action-retry@v1
uses: CloudCompilers/gh-action-retry@v1
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the new org name not work with this?

Copy link
Author

Choose a reason for hiding this comment

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

Whoops, yes it would/should. I missed this, thanks!

@ghost
Copy link
Author

ghost commented Jan 9, 2023

I'm not clear what this change enables us to do - can you describe a little more?

Sorry about that! It lets us invoke these integ tests from another workflow, and specifically it'll let us invoke them (but with a different sample-apps repo) from the klotho-pro repo. That means that we won't have to maintain two copies of that big integtest.yaml file: we'll have just this one, and then each repo (klotho and klotho-pro) can invoke it to run its respective tests.

@gordon-klotho
Copy link
Contributor

That means that we won't have to maintain two copies of that big integtest.yaml

You mean like using a uses?

@ghost
Copy link
Author

ghost commented Jan 9, 2023

That means that we won't have to maintain two copies of that big integtest.yaml

You mean like using a uses?

Yup, exactly. The pro repo will have a trivial workflow almost identical to this PR's nightly-integ-tests.yaml

@github-actions
Copy link

github-actions bot commented Jan 9, 2023

Package Line Rate Health
github.com/klothoplatform/klotho/pkg/analytics 2%
github.com/klothoplatform/klotho/pkg/annotation 24%
github.com/klothoplatform/klotho/pkg/core 21%
github.com/klothoplatform/klotho/pkg/env_var 82%
github.com/klothoplatform/klotho/pkg/exec_unit 41%
github.com/klothoplatform/klotho/pkg/infra/kubernetes 58%
github.com/klothoplatform/klotho/pkg/infra/kubernetes/helm 52%
github.com/klothoplatform/klotho/pkg/input 63%
github.com/klothoplatform/klotho/pkg/lang 37%
github.com/klothoplatform/klotho/pkg/lang/dockerfile 0%
github.com/klothoplatform/klotho/pkg/lang/golang 9%
github.com/klothoplatform/klotho/pkg/lang/javascript 46%
github.com/klothoplatform/klotho/pkg/lang/python 60%
github.com/klothoplatform/klotho/pkg/lang/yaml 0%
github.com/klothoplatform/klotho/pkg/logging 7%
github.com/klothoplatform/klotho/pkg/multierr 95%
github.com/klothoplatform/klotho/pkg/provider/aws 60%
github.com/klothoplatform/klotho/pkg/runtime 75%
github.com/klothoplatform/klotho/pkg/static_unit 32%
github.com/klothoplatform/klotho/pkg/validation 73%
Summary 42% (3586 / 8582)

@ghost ghost merged commit 108002f into main Jan 9, 2023
@ghost ghost deleted the reusable-integ-test branch January 9, 2023 18:57
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants