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

chore(operations): Add test harness GH action #2028

Merged
merged 2 commits into from
Mar 23, 2020
Merged

Conversation

binarylogic
Copy link
Contributor

@binarylogic binarylogic commented Mar 10, 2020

Closes #1416

This adds a test-harness.yml Github Action that automates running the test harness on the current branch. Things left to do:

Signed-off-by: binarylogic <bjohnson@binarylogic.com>

# Only run if we're invoked with a new command comment on a pull request.
if: |
github.event_name == 'issue_comment' && github.event.action == 'created'
Copy link
Contributor

@Hoverbear Hoverbear Mar 10, 2020

Choose a reason for hiding this comment

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

How about edited too so we can easily rerun tests without creating spam? Or can we use reactions to avoid extra notifications entirely?

Copy link
Contributor

Choose a reason for hiding this comment

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

Test harness creates a comment with the results anyways, so this doesn't completely eliminate the notifications spam. If was solved completely - I'd go for it, but since it doesn't achieve the cover the issue - I think we better not attach any functionality to the edits yet.

I was thinking, in the future, when github actions provide support for it, we could support properly handling edit events to cancel and then rerun the test harness invocation initiated by a particular comment. This would work best if you one made a typo in the command, and want to correct it.

Overall, from the general PR discussion flow perspective, I think it'll work better if we have a run per /test comment, rather than a single /test comment to correspond to multiple runs. The notifications might be a problem, but if they are I'dd address it separately. If we really want an easy way to restart a run - I'd look into doing something with reactions - but it's worth a separate task.

@Hoverbear
Copy link
Contributor

Can we see a demo?

@binarylogic
Copy link
Contributor Author

@MOZGIII needs to add the final touches here. Tomorrow we should be able to demo. Once this is working, I'd like to merge to master and then rebase #1922.

@Hoverbear
Copy link
Contributor

Looking forward to adopting!

@MOZGIII
Copy link
Contributor

MOZGIII commented Mar 11, 2020

We'll have to merge this in before we can do a demo, because issue_comment events are attached to master branch. But there is vectordotdev/vector-test-harness-github-actions-test-repo#3 where you can try this out. It might be at a WIP state though.

This PR is more of a placeholder, and we'll force push the latest version into here after we make it work at https://github.com/timberio/vector-test-harness-github-actions-test-repo. It's a shame it's not possible to apply regular PR flow here, but it's a limitation of how we're using Github Actions.

@MOZGIII
Copy link
Contributor

MOZGIII commented Mar 11, 2020

Here's a demo PR that simulates tokio-compat: vectordotdev/vector-test-harness-github-actions-test-repo#13

@MOZGIII
Copy link
Contributor

MOZGIII commented Mar 14, 2020

Good news regarding Github Actions in general: the act tool was fixed during this last month, and we can now use it to invoke test harness locally as if via Github Actions. Of course, it's only useful for the local development of the Github Action itself, as it's not a sane way of invoking test harness if you want to run it locally, but still!

Here's how to do it:

$ cat <<EOF > event.json
{
  "action": "created",
  "comment": {
    "body": "/test qwe"
  },
  "issue": {
    "pull_request": {}
  }
}
EOF
$ act issue_comment -v -e event.json
[Test Harness/test-harness] 🚀  Start ...

Don't expect it to fully run with such a sparse event file and without secrets, but at least it'll do something. act from a month ago would straight up fail at reading workflow!
This should speed up the iterations on Github Actions significantly!

@binarylogic
Copy link
Contributor Author

From planning: we can merge this as is, we'll just need to invoke tests individually. We can follow up with optimizations.

Signed-off-by: MOZGIII <mike-n@narod.ru>
@MOZGIII
Copy link
Contributor

MOZGIII commented Mar 23, 2020

I'm merging if everyone's ok with it!

@binarylogic
Copy link
Contributor Author

:shipit:

@MOZGIII MOZGIII merged commit ce7bdba into master Mar 23, 2020
@binarylogic binarylogic deleted the test-harness-action branch April 24, 2020 20:38
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.

Automate test harness and integrate into build process
3 participants