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

Ccpansys/solutions e2e testing #384

Merged
merged 14 commits into from
Oct 30, 2023
Merged

Conversation

ccpansys
Copy link
Contributor

This PR adds a workflow file that will trigger the solution templates e2e tests on each PR that modifies one of the Solution templates.

@github-actions github-actions bot added the maintenance Package and maintenance related label Oct 26, 2023
@ccpansys ccpansys assigned iazehaf and unassigned iazehaf Oct 26, 2023
@ccpansys ccpansys requested a review from iazehaf October 26, 2023 10:13
@ccpansys ccpansys requested a review from a team as a code owner October 26, 2023 12:46
Copy link
Contributor

@nick-marnik nick-marnik left a comment

Choose a reason for hiding this comment

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

Just a few minor suggestions

.github/workflows/solutions_ci.yml Outdated Show resolved Hide resolved
.github/workflows/solutions_ci.yml Outdated Show resolved Hide resolved
.github/workflows/solutions_ci.yml Outdated Show resolved Hide resolved
.github/workflows/solutions_ci.yml Show resolved Hide resolved
.github/workflows/solutions_ci.yml Outdated Show resolved Hide resolved
@ccpansys
Copy link
Contributor Author

@nick-marnik, @jacobevansgit: Thank you for your feedback!

Copy link
Contributor

@nick-marnik nick-marnik left a comment

Choose a reason for hiding this comment

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

NASA LGTM

Much cleaner - thanks for the quick turnaround on the changes! 👍

.github/workflows/solutions_ci.yml Show resolved Hide resolved
@ccpansys ccpansys merged commit edbee32 into main Oct 30, 2023
53 checks passed
@ccpansys ccpansys deleted the ccpansys/solutions-e2e-testing branch October 30, 2023 08:13
@nick-marnik
Copy link
Contributor

@ccpansys This build triggered on a different PR and it looks like the secret may not exist: https://github.com/ansys/ansys-templates/actions/runs/6746000959/job/18338968194

Based on line 9, are you sure that ${{ secrets.ANSYS_SOLUTIONS_E2E_TESTS }} exists on this repo? I don't have admin access to check myself 😢

@ccpansys
Copy link
Contributor Author

ccpansys commented Nov 3, 2023

@nick-marnik That is surprising. The workflow has run with that token before, so it must exist (or it did, at least). And I am afraid I don't have permissions to check, either. @iazehaf, can you help us figure this one out?

@nick-marnik
Copy link
Contributor

I just checked and @ccpansys is correct, this build has run successfully in the past: https://github.com/ansys/ansys-templates/actions/workflows/solutions_ci.yml

I am just speculating, but I suspect this may be related to my permissions on this repo. Comparing the successful builds to my failed build, the difference is that my PR is based off of a branch from my personal fork of this repository. At one point in time, I correctly had write permissions for this repo and was able to push my branch directly. However, somewhere along the way of the public/private migrations of this repo, my permissions were broken and I was relegated to using a fork.

Paging @jacobevansgit for comment

PR in question: #365

@jacobevansgit
Copy link

Sorry @nick-marnik @ccpansys I am also not admin of this org. Please reach out to Pyansys colleagues.

However after looking at this PR again, I am wondering if we should have this open repo signalling to start tests inside our internal org. The more appropriate way for this to scale would be to have the tests repo "listen" to this repository (ansys-templates) for any changes and then trigger if any are found.

Updating the ansys-templates each time for a future 5-10-50 repositories that need testing after this repo has changes, is not appropriate IMO.

@nick-marnik
Copy link
Contributor

Sorry @nick-marnik @ccpansys I am also not admin of this org. Please reach out to Pyansys colleagues.

However after looking at this PR again, I am wondering if we should have this open repo signalling to start tests inside our internal org. The more appropriate way for this to scale would be to have the tests repo "listen" to this repository (ansys-templates) for any changes and then trigger if any are found.

Updating the ansys-templates each time for a future 5-10-50 repositories that need testing after this repo has changes, is not appropriate IMO.

I don't disagree with your premise, but unfortunately I am not aware of a way to achieve this behavior. Currently, workflow_run triggers only work within the same repository, not between repos.

@jorgepiloto Do you have any idea what could be happening with the secret mentioned above?

@jorgepiloto
Copy link
Member

Hi @nick-marnik. I can confirm that the secret ANSYS_SOLUTIONS_E2E_TESTS exists in this repository. It was updated two weeks ago. Have you authorized the TOKEN to operate with the Solution Applications org?

@nick-marnik
Copy link
Contributor

Hi @nick-marnik. I can confirm that the secret ANSYS_SOLUTIONS_E2E_TESTS exists in this repository. It was updated two weeks ago. Have you authorized the TOKEN to operate with the Solution Applications org?

I don't have access to see this, either, but I assume it is configured correctly because it works for some builds.

Are my permissions on this repo correct? I am curious if testing with a non-forked branch would yield different results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Package and maintenance related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants