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

ci: refine PR tests to exclude unchanged complex examples #778

Open
glasnt opened this issue Dec 9, 2024 · 5 comments
Open

ci: refine PR tests to exclude unchanged complex examples #778

glasnt opened this issue Dec 9, 2024 · 5 comments

Comments

@glasnt
Copy link
Contributor

glasnt commented Dec 9, 2024

TL;DR

Identified in 773, debugged in #777

After changes in #669, there are still samples that aren't pruned. This increases the CI execution time

Expected behavior

No response

Observed behavior

No response

Terraform Configuration

build/int.cloudbuild.yaml

Terraform Version

cloud-foundation-toolkit latest

Additional information

No response

@glasnt
Copy link
Contributor Author

glasnt commented Dec 9, 2024

Example remaining folders:

looker/ et al contains test.yaml files, so samples are validated but not executed (slight increase to CI exec time)
functions/ contains reference code (.js, package.json), so samples are executed completely (and failing in 777 exec)

@glasnt
Copy link
Contributor Author

glasnt commented Dec 10, 2024

@apeabody confirming your intent on 669, this comment was added to the code:

Remove leaf folders without changes or non-tf resources

Do you intend to always test samples with test.yaml files? Always samples with other artifacts?

@apeabody
Copy link
Contributor

@apeabody confirming your intent on 669, this comment was added to the code:

Remove leaf folders without changes or non-tf resources

Do you intend to always test samples with test.yaml files? Always samples with other artifacts?

Hey @glasnt! I'd have to refresh my memory, but the intention was to avoid deleting non-TF test artifacts that are (commonly) in sub-directories below the TF code itself. Likely the logic can/should be improved. :) At the very least it should be safe to delete folders with test.yaml as those are not per say test artifacts.

@glasnt
Copy link
Contributor Author

glasnt commented Dec 11, 2024

There may be a new general solution on this coming from my team, but in the mean time, I will address some of the more flaky resource-heavy tests as to try and streamline the process for other PRs

@apeabody
Copy link
Contributor

There may be a new general solution on this coming from my team, but in the mean time, I will address some of the more flaky resource-heavy tests as to try and streamline the process for other PRs

As a quick fix, I was thinking we could first run the original logic to purge root level directories with zero changes, and then depend on the new logic just to clean the remaining directories further. Likely still some edge cases, but that would give us at least the best of both right in the meantime.

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

No branches or pull requests

2 participants