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

HARMONY-1874: As a harmony service provider, I want to execute a regression test based on receiving a deployment event in the harmony-regression-tests repo #98

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

ygliuvt
Copy link
Member

@ygliuvt ygliuvt commented Sep 30, 2024

Description

As a harmony service provider, I want to execute a regression test based on receiving a deployment event in the harmony-regression-tests repo.

This PR is a proof of concept. It is based on @owenlittlejohns' fork https://github.com/owenlittlejohns/harmony-regression-tests. It only set up one real service deployment event hoss which will kick off the hoss regression service test when hoss service is deployed. A fake event that has been set up is harmony-service-example which will kick off the variable subsetter regression test. The configuration of github event type and regression testing will be added in subsequent tickets and PRs.

The personal access token, secrets and variables that are needed to run the github action on this repo will be added and tested once the PR is merged to baseline. An example setup has been tested in https://github.com/ygliuvt/harmony-regression-tests.

Jira Issue ID

HARMONY-1874

Local Test Steps

TBD. See https://github.com/ygliuvt/harmony-regression-tests.

PR Acceptance Checklist

  • Acceptance criteria met
  • Tests added/updated (if needed) and passing
  • Documentation updated (if needed)

owenlittlejohns and others added 24 commits September 30, 2024 15:53
Copy link
Member

@flamingbear flamingbear left a comment

Choose a reason for hiding this comment

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

Overall I like this approach a lot. It's nice that a user can just run a any test with any environment. (not SIT yet because of permissions still?)

I don't love the duplication in each test specific workflow.

Would it be possible to have a workflow that intercepts all repository_dispatch events and then calls a generic test-suite with the correct parameters? It doesn't look like there's too many of them.

It seems like the overall maintenance would be easier for someone to add new tests to the repository. Of course as I'm typing "you'd just add a configuration for your test" the same could be said that you'd copy an existing workflow, but when that workflow has to change, you'd have to update every specific tests' workflow.

I am unclear exactly how I would intercept a repository_event to dispatch a workflow in a different repo (say a particular DAAC's regressions) still. I think that's something we would want to be able to do.

I hate to suggest a meeting, but should we try to discuss an architecture for TRT-532. Not sure who to involve. I'll follow up on slack

.github/workflows/variable-subsetter-test-suite.yml Outdated Show resolved Hide resolved
Copy link
Member

@owenlittlejohns owenlittlejohns left a comment

Choose a reason for hiding this comment

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

The email notification additions to the workflows are great!

Generally, I'm still a bit wary about a model of having to add a whole new workflow for every test suite - it feels like a fair amount of overhead for a service/data provider, and the current examples are highly duplicated between each other. (On the one hand, it's setting off my DRY alarms, on the other maybe that's good to allow easy mimicking?)

I think the main conversation we need to have is what we are trying to optimise for. My opening suggestion would be that we should make things as simple as possible for a new service developer or data provider to hook up a new regression test suite. I'd maybe suggest that setting up a whole workflow YAML file (even if it's largely a copy-and-paste exercise) might be a bit intimidating to folks who haven't dabbled in GH workflows/actions before.

.github/workflows/hoss-test-suite.yml Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
.github/workflows/run-target-test-suite.yml Outdated Show resolved Hide resolved
Comment on lines +2 to +3
# It can be run manually run on github Actions or invoked through
# the github Action workflow api.
Copy link
Member

Choose a reason for hiding this comment

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

Can it? Is it a permission thing or an understanding thing that I can't do that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I lied. I took out the workflow dispatch. I am going to add it back.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok. Re-enabled workflow dispatch. Now you should be able to run manually if you have permission.

Copy link
Member

@owenlittlejohns owenlittlejohns left a comment

Choose a reason for hiding this comment

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

I think this is looking good. I just had two quick questions, but no showstoppers.

# Note: A workflow_call event can only have inputs of type boolean, number or
# string.
name: Run Jupyter notebook based test suite

Copy link
Member

Choose a reason for hiding this comment

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

Was the work to make a more dynamic run-name going to be included in another PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

oh, I misunderstood run-name. I have added it.

steps:
- name: Get TO_EMAIL
run: |
SERVICE_EMAILS=$(echo '${{ vars.SERVICES_EMAILS_CONFIG }}' | jq -r '."${{ github.event.action || inputs.service-name }}"')
Copy link
Member

Choose a reason for hiding this comment

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

Without giving away anything sensitive, could you describe in a comment what the SERVICES_EMAILS_CONFIG looks like? (Or someone could add me as a repository admin so I can just go and take a look)

Copy link
Member Author

Choose a reason for hiding this comment

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

Here is an example of the SERVICES_EMAILS_CONFIG:

{
  "harmony-service-example": "[email protected]",
  "hoss": "[email protected],[email protected],[email protected]"
}

There is also another configuration: TO_EMAIL_DEFAULT with value of a list of emails, that will be used if the service is not defined in the SERVICES_EMAILS_CONFIG.
Note: Currently NASA blocks the emails sent from my personal gmail account that I used for demo purpose. We will file a ticket and get the email with NASA email addresses working later.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have added you and Matt as collaborators in my fork. Hopefully, that will give you guys permission to see things.

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

Successfully merging this pull request may close these issues.

3 participants