Open source tasks + tests refactoring

[s-samadi]

What does this pull request accomplish?

• Refactored tests to stub and mock instead of calling real environments. As a result I had to refactored some of the code to accept Client parameters. This results in a better function signature all around because the Client becomes dynamic rather than statically assigned in the method. It also removes duplication, for example, in the delete branch logic we were duplicating the creation of the client by having logic as follows:

client, err = api.defaultRESTClient()
...
getRepos() 
....
func getRepos(..) {
callApi(...)
}
....
func callApi(...) {
client, err = api.defaultRESTClient()
}

This pattern is easy to fall into if the Client is not parameterized. Therefore, the refactor addressed both test stubbing and logical errors.

For the test stubbing, I created a mock TestClient and stubbed API responses.

• Added the required documentation for open sourcing the repos (e.g code of conduct, security, support) etc.
• Added a linter to run on the workflow
• Changed the testing workflow to also output test coverage
• Added CodeQL workflow
• Added Dependency Review

[github-advanced-security]

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

[therealkujo]

This results in a better function signature all around because the Client becomes dynamic rather than statically assigned in the method.

Could you elaborate more on this? I don't think I understand why it is better to be invoked elsewhere since there is nothing different in how it is created when it is made in any other function.

It also removes duplication, for example, in the delete branch logic we were duplicating the creation of the client

Actually the delete branch logic hasn't been refactored so it shouldn't be creating a client anyways. That codebase wasn't touched as part of my first round refactoring which is why it has the old logic in it

[s-samadi]

@therealkujo replies below

Could you elaborate more on this? I don't think I understand why it is better to be invoked elsewhere since there is nothing different in how it is created when it is made in any other function.

Sure. It was not possible to stub API responses because the GitHub Client was assigned inside of the functions. By removing it and passing it in as a parameter it becomes more dynamix - i.e you can pass in a stubbed client or it can be the GitHub Client. For testing, the TestClient is an interface that has implemented a stubbed Request method which allows for mocked responses. This gets passed into the methods as a parameter.

Actually the delete branch logic hasn't been refactored so it shouldn't be creating a client anyways. That codebase wasn't touched as part of my first round refactoring which is why it has the old logic in it

Sure, it doesn't change the fact that client, err = api.defaultRESTClient() was executed twice. Hence the duplication comment. To my point, by having it parameterised as something that gets passed into the common functions, we can better ensure that the client is only initiated once in the command.

[therealkujo]

Thanks for doing this! There are some minor grammar changes I found.

Other than this, I wanted to get your thoughts before approving this. I really like that we are mocking everything but also wonder if we should have the ability to do an actual run as well? In the event the GitHub APIs change, how would we be able to catch that in our tests? Might be nice to be able to periodically run our tests live on the production branch to ensure that nothing is broken but be able to use all the mocks for PR reviews.

[therealkujo]

🎉 Thanks for working on this!