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

WIP use GH action to detect which service tests to run; also run [packagist] #5129

Closed
wants to merge 2 commits into from

Conversation

chris48s
Copy link
Member

@chris48s chris48s commented May 23, 2020

refs #2545
refs #1936

I've been using github actions a fair bit on some other projects and I thought I'd revisit this idea.

The implementation I've got so far continues to allow us to specify tests to run in the PR title (as we do now). This is mainly useful for dependabot PRs where we aren't modifying any service or test code but still want to run some service tests.

It also uses the GH actions API to detect any services/**/*.(service|tester).js files modified and attempts to run relevant service tests based on the path. At the moment, the way I'm doing this is fairly accurate, but I've realised that there are some cases where there's a bit of a mismatch between the directory names and service/tester names. For example SubredditSubscribers lives in /reddit, most of the tester names don't contain the - character, except snap-ci and f-droid, which do. Also some of the services that only export a redirector don't match with the path name. One thing we could do is rename some stuff to make it more consistent, but there's not really anything to stop further discrepancies appearing in future.

I think if we want to take this approach forward, what I'll probably do is write a second test loader that works based on providing paths and then use that to make this more reliable (but retain the current name-based loading for services specified in the PR title). This is a bit more involved though, so I figured I'd get some feedback on the WIP before I embark on that journey. There will also be some work on:

  • docs
  • testing
  • removing Circle CI config
  • moving secrets/keys from Circle env vars GH Actions
  • updating branch protection rules, etc

to fully finish this..

Thoughts?

The service tests running on this PR are arbitrary - its just to demonstrate the action actually running some tests

- modify pull-request-services-cli to pick up services based on
    - files modified in PR diff or
    - PR title
- set up GH Actions to run the serivce tests
@chris48s chris48s added the developer-experience Dev tooling, test framework, and CI label May 23, 2020
@chris48s
Copy link
Member Author

boom: https://github.com/badges/shields/pull/5129/checks?check_run_id=702053286
Thinking about it, I suspect the ci/circleci: services builds will probably fail, as I've not disabled them but I've deleted some code they need..

@shields-ci
Copy link

Warnings
⚠️ This PR modified service code for hexpm but not its test code.
That's okay so long as it's refactoring existing code.
⚠️ This PR modified service code for pub but not its test code.
That's okay so long as it's refactoring existing code.
Messages
📖 ✨ Thanks for your contribution to Shields, @chris48s!

Generated by 🚫 dangerJS against 7080dde

@calebcartwright
Copy link
Member

I'll preface by saying that I've had generally positive experiences with GHA so far (though it's admittedly still fairly immature compared to the more established CI platforms) and am a fan.

Is the thinking that long term we could migrate our CI/PR validation from Circle to GHA, or just moving the service test run jobs to GHA while keeping the rest in Circle?

@chris48s
Copy link
Member Author

I'll preface by saying that I've had generally positive experiences with GHA so far (though it's admittedly still fairly immature compared to the more established CI platforms) and am a fan.

That exactly summarises my opinion on it too :)

Is the thinking that long term we could migrate our CI/PR validation from Circle to GHA, or just moving the service test run jobs to GHA while keeping the rest in Circle?

Certainly at the moment I'd only be looking to move the service tests and leave everything else running on Circle CI. The reason I think GH actions is a good fit for this specific job is because its really easy to get an array of all the files changed in this PR in the CI build or otherwise interrogate the PR (for example, you'll notice I've been able to completely bin all the code in infer-pull-request.js). There's no reason why we couldn't do that in Circle instead.. its just a bit more of a faff.

I don't see a problem with having some tests run in GH Actions and some in Circle, but if we have good experiences with it we could also look at moving more stuff in future.

@chris48s
Copy link
Member Author

this eventually got turned into #8421

@chris48s chris48s closed this Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer-experience Dev tooling, test framework, and CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants