-
Notifications
You must be signed in to change notification settings - Fork 8
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
Functional tests for local and remote PR creation. --dry-run? #56
Comments
@mnuttall do you think these should instead be unit tests (we have https://github.com/rhd-gitops-example/services/blob/f020ad9125c422a4246688e8e194e429df8905ec/pkg/git/repository_test.go which accepts a TEST_GITHUB_TOKEN for example), or are you thinking of introducing Bash tests that we provide a token for? The Bash example would be more realistic but harder to automate unless we get a functional GitHub ID for Travis, perhaps whoever picks this up should do/investigate both? For a dry run flag, lemme see if I understand this right - would this be the same as a user clicking the "compare and pull request' button on GitHub when they've just pushed to a branch for a forked repository? I'm sure it's a well-formed URL we could generate, we could even spawn a browser with it up instead of just telling users to paste it - this would assume the branch has already been pushed and both are real repositories on GitHub (so, not "local filesystem to real GitHub repo", I think). |
@JustinKuli added a whole suite of tests under https://github.com/rhd-gitops-example/services/pull/112/files#diff-5aaf8affd544566465993c820866da3d that should cover the kind of failures that caused me to raise this issue. Closing this issue in favour of #112. |
#32 looked good but it broke promotion from a local repository. We lack the tests for the main Promote() method that should have caught it.
Promote() results in the generation of a PullRequest. A test for this method might need to generate pull requests for both local and remote scenarios, check that they look right, and then close the PRs.
An alternative would be to introduce a --dry-run CLI flag so that we could verify what the PR would look like, were we to raise it, but stop short of doing so. This might make the tests a bit more reliable.
The text was updated successfully, but these errors were encountered: