Skip to content
This repository has been archived by the owner on Mar 22, 2024. It is now read-only.

Skip tests for docs folders #281

Merged
merged 2 commits into from
May 16, 2023
Merged

Skip tests for docs folders #281

merged 2 commits into from
May 16, 2023

Conversation

faisal-memon
Copy link
Contributor

@faisal-memon faisal-memon commented May 16, 2023

Our mandatory checks will fail if there are docs only changes. The helm-chart-ci.yaml skips these files, so the workflows never run, and the mandatory checks are stuck waiting for results.

This adds a parallel workflow that runs fake tests for docs changes. So that docs only changes can pass the required status checks. It uses paths-ignore instead of paths. This is documented here.

Signed-off-by: Faisal Memon <fymemon@yahoo.com>
Co-authored-by: Marco Franssen <marco.franssen@gmail.com>
Signed-off-by: Faisal Memon <fymemon@yahoo.com>
@kfox1111
Copy link
Contributor

Hmm.. Interesting problem...

Would it make sense to just always run the tests in this case too? Remove: https://github.com/spiffe/helm-charts/blob/main/.github/workflows/helm-chart-ci.yaml#L7-L15

It would mean the test suite would run on doc only changes too, but there has not been very many of those pr's so maybe the simplification would be worth it?

@faisal-memon
Copy link
Contributor Author

Hmm.. Interesting problem...

Would it make sense to just always run the tests in this case too? Remove: https://github.com/spiffe/helm-charts/blob/main/.github/workflows/helm-chart-ci.yaml#L7-L15

It would mean the test suite would run on doc only changes too, but there has not been very many of those pr's so maybe the simplification would be worth it?

Im fine either way though slightly prefer not running tests for docs only changes.

@kfox1111
Copy link
Contributor

Ok. There's stuff in here that is kind of duplication from other things and needs to be synchronized between the two or else the pipeline breaks. so maybe some changes could be added to make that stuff common so we can change it just in one place?

Since things are broken, I'm ok with that kind of thing being done in a followup pr though.

@faisal-memon
Copy link
Contributor Author

Ok. There's stuff in here that is kind of duplication from other things and needs to be synchronized between the two or else the pipeline breaks. so maybe some changes could be added to make that stuff common so we can change it just in one place?

Since things are broken, I'm ok with that kind of thing being done in a followup pr though.

I agree. Is there a good way to do that? I always end up with a lot of duplication with actions.

@faisal-memon faisal-memon merged commit 72596ae into spiffe:main May 16, 2023
40 checks passed
@kfox1111
Copy link
Contributor

I agree. Is there a good way to do that? I always end up with a lot of duplication with actions.

No idea. :/

I'm still pretty new to actions.

The find could maybe be broken out to a script and shared that way.

@kfox1111
Copy link
Contributor

Might be able to use an environment variable from a file along with maybe the fromJson thing refereed to here: https://stackoverflow.com/questions/61919141/read-json-file-in-github-actions
for building the path list.

There may be simpler ways though. Maybe @marcofranssen knows better way?

marcofranssen added a commit that referenced this pull request May 25, 2023
* c1c5b11 Merge pull request #306 from spiffe/remove-1.21
* 0df45e3 Fix up docs
* ed038fe Upgrade to spire 1.6.4 (#308)
* dc5d9cf Fix root README.md
* e4447fd Upgrade Tornjak to new image v1.2.1 (#299)
* 69f402e Update docs
* 38d51d5 Apply suggestions from code review
* a1ba235 Update docs
* 1922085 Fix hooks for K3s (#305)
* 4fb549e Remove 1.21.x testing
* 88efc77 Allow to use spire-server as an upstream authority (#304)
* 0ba0388 Add support for spire-server ingress (#68)
* 4777a30 Bump test chart dependencies (#301)
* 00c2c1a Fix the generated pr so that it runs jobs too (#303)
* dd1ad49 Update images for cve's found by the cronjob (#290)
* 1c69470 Updated Tornjak documenation with Not-for-production labels (#297)
* 7809637 Merge pull request #296 from spiffe/dependabot/github_actions/helm/kind-action-1.7.0
* e61ed17 Merge pull request #295 from spiffe/dependabot/github_actions/sigstore/cosign-installer-3.0.5
* 9975e58 Merge pull request #245 from spiffe/tags
* 7bb7ece Bump helm/kind-action from 1.6.0 to 1.7.0
* f1623a5 Bump sigstore/cosign-installer from 3.0.4 to 3.0.5
* f8db5a3 Fix Tornjak persistence issue (#294)
* b30b412 Tornjak reuse spire-lib.cluster-domain macro (#292)
* 90c9eb5 Fix kubectl-image macro to handle version deprecation
* 300d1cc Apply deprecation of image.version to Tornjak
* d850486 Instead of removing version, first deprecate version
* 59e422b Add documentation for all image.tag values
* d1f3cdb Switch image.version to image.tag
* 31ce704 Cleanup maintainer handbook (#287)
* a2da943 Remove manual dispatch from dummy workflow (#288)
* 807558b Bump helm/kind-action from 1.5.0 to 1.6.0 (#285)
* 3df67db Bump sigstore/cosign-installer from 3.0.3 to 3.0.4 (#286)
* 5505d41 Merge pull request #283 from spiffe/additional-k8s-native-feature-tornjak-frontend
* 391f093 Allow to configure topologySpreadConstraints for tornjak-frontend
* 5cc26d3 Allow to configure tolerations for tornjak-frontend
* 3537161 Allow to configure affinity for tornjak-frontend
* aed6fdf Use the correct kubectl for the cluster (#248)
* ee43c5e Add nodeSelector for tornjak
* fc13cbd Merge pull request #234 from spiffe/tornjak
* ed472aa Update documentation
* a11cfc9 Allow to define the resources for tornjak backend
* 382e0d4 Upgrade Tornjak image to version v1.2.0  (#259)
* 657c460 Update charts/spire/charts/tornjak-frontend/templates/service.yaml
* 7521caf Update charts/spire/charts/spire-server/templates/tornjak-config.yaml
* b64c352 Update charts/spire/charts/spire-server/templates/tests/test-tornjak-connection.yaml
* 6ddf6ab Improve tornjak docs (#276)
* 80d34f0 Use common post-install scripts for testing
* f5efa0c Remove dead macros
* bd86518 Fixing shellcheck
* 91bdea2 Provide minimal resources to prevent accidental crashes due to resource exhaustion
* 1675997 Tornjak global image fix (#228)
* 5e827ee Add Tornjak Tests (#220)
* bdba97b Add empty directory to Tornjak to support npm cache (#224)
* da186c5 Split Tornjak Frontend into separate subchart (#179)
* 6d22126 Add Tornjak
* 2669d8b Add maintainer's handbook. (#265)
* 72596ae Skip tests for docs folders (#281)
* 7c71738 Bump test chart dependencies (#279)
* 05addae Add json to test path (#280)
* 8d9b734 Switch the spire tests to always run (#250)

Signed-off-by: Marco Franssen <marco.franssen@gmail.com>
marcofranssen added a commit that referenced this pull request May 25, 2023
* c1c5b11 Merge pull request #306 from spiffe/remove-1.21
* 0df45e3 Fix up docs
* ed038fe Upgrade to spire 1.6.4 (#308)
* dc5d9cf Fix root README.md
* e4447fd Upgrade Tornjak to new image v1.2.1 (#299)
* 69f402e Update docs
* 38d51d5 Apply suggestions from code review
* a1ba235 Update docs
* 1922085 Fix hooks for K3s (#305)
* 4fb549e Remove 1.21.x testing
* 88efc77 Allow to use spire-server as an upstream authority (#304)
* 0ba0388 Add support for spire-server ingress (#68)
* 4777a30 Bump test chart dependencies (#301)
* 00c2c1a Fix the generated pr so that it runs jobs too (#303)
* dd1ad49 Update images for cve's found by the cronjob (#290)
* 1c69470 Updated Tornjak documenation with Not-for-production labels (#297)
* 7809637 Merge pull request #296 from spiffe/dependabot/github_actions/helm/kind-action-1.7.0
* e61ed17 Merge pull request #295 from spiffe/dependabot/github_actions/sigstore/cosign-installer-3.0.5
* 9975e58 Merge pull request #245 from spiffe/tags
* 7bb7ece Bump helm/kind-action from 1.6.0 to 1.7.0
* f1623a5 Bump sigstore/cosign-installer from 3.0.4 to 3.0.5
* f8db5a3 Fix Tornjak persistence issue (#294)
* b30b412 Tornjak reuse spire-lib.cluster-domain macro (#292)
* 90c9eb5 Fix kubectl-image macro to handle version deprecation
* 300d1cc Apply deprecation of image.version to Tornjak
* d850486 Instead of removing version, first deprecate version
* 59e422b Add documentation for all image.tag values
* d1f3cdb Switch image.version to image.tag
* 31ce704 Cleanup maintainer handbook (#287)
* a2da943 Remove manual dispatch from dummy workflow (#288)
* 807558b Bump helm/kind-action from 1.5.0 to 1.6.0 (#285)
* 3df67db Bump sigstore/cosign-installer from 3.0.3 to 3.0.4 (#286)
* 5505d41 Merge pull request #283 from spiffe/additional-k8s-native-feature-tornjak-frontend
* 391f093 Allow to configure topologySpreadConstraints for tornjak-frontend
* 5cc26d3 Allow to configure tolerations for tornjak-frontend
* 3537161 Allow to configure affinity for tornjak-frontend
* aed6fdf Use the correct kubectl for the cluster (#248)
* ee43c5e Add nodeSelector for tornjak
* fc13cbd Merge pull request #234 from spiffe/tornjak
* ed472aa Update documentation
* a11cfc9 Allow to define the resources for tornjak backend
* 382e0d4 Upgrade Tornjak image to version v1.2.0  (#259)
* 657c460 Update charts/spire/charts/tornjak-frontend/templates/service.yaml
* 7521caf Update charts/spire/charts/spire-server/templates/tornjak-config.yaml
* b64c352 Update charts/spire/charts/spire-server/templates/tests/test-tornjak-connection.yaml
* 6ddf6ab Improve tornjak docs (#276)
* 80d34f0 Use common post-install scripts for testing
* f5efa0c Remove dead macros
* bd86518 Fixing shellcheck
* 91bdea2 Provide minimal resources to prevent accidental crashes due to resource exhaustion
* 1675997 Tornjak global image fix (#228)
* 5e827ee Add Tornjak Tests (#220)
* bdba97b Add empty directory to Tornjak to support npm cache (#224)
* da186c5 Split Tornjak Frontend into separate subchart (#179)
* 6d22126 Add Tornjak
* 2669d8b Add maintainer's handbook. (#265)
* 72596ae Skip tests for docs folders (#281)
* 7c71738 Bump test chart dependencies (#279)
* 05addae Add json to test path (#280)
* 8d9b734 Switch the spire tests to always run (#250)

Signed-off-by: Marco Franssen <marco.franssen@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants