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

Add integration tests for important commands #210

Merged
merged 9 commits into from
Nov 7, 2024
Merged

Conversation

pjcdawkins
Copy link
Contributor

@pjcdawkins pjcdawkins commented Oct 7, 2024

Tested on the GitLab side

Rationale:

  • Go seems to be fine at running these kinds of tests, and it removes the need to worry about Python and Selenium, which have not been working (on GitLab), and which represent unnecessary barriers to testing on other platforms.
  • Go allows easy mocking of API servers to test particular commands.

Awkwardnesses were:

  • I set cmd.Dir = os.TempDir() when running a command: so that the legacy CLI doesn't do unpredictable things with the "current project".
  • I added the chi router package as a dependency for the test mocks. That probably does not affect the production build (on this branch, it's about 1KB smaller for me).

Follow up steps could be:

  • Build or download PHP and the Legacy CLI on GitHub Actions, so that the integration tests can be run there.
  • Test on multiple operating systems, such as Linux, Windows and MacOS.

@pjcdawkins pjcdawkins force-pushed the use-golang-tests branch 3 times, most recently from 6eb7a55 to ac8ad3e Compare October 7, 2024 15:02
@pjcdawkins pjcdawkins marked this pull request as ready for review October 7, 2024 15:12
@pjcdawkins pjcdawkins changed the title Use Go for integration tests; test output exactly Use Go for integration tests Oct 8, 2024
@pjcdawkins pjcdawkins changed the title Use Go for integration tests Add integration tests for important commands Oct 9, 2024
@pjcdawkins pjcdawkins force-pushed the use-golang-tests branch 7 times, most recently from 5a59989 to a961dba Compare October 12, 2024 09:44
@pjcdawkins pjcdawkins force-pushed the use-golang-tests branch 3 times, most recently from 4a7b086 to 9ab9d6d Compare October 16, 2024 13:18
Copy link
Member

@akalipetis akalipetis left a comment

Choose a reason for hiding this comment

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

Really nice one, this will help us on so many fronts 🎉

I have just a couple of questions, the rest of the code is generally looking great, I put most of my focus on the "testing infra" code.

Makefile Outdated Show resolved Hide resolved
tests/tests.go Outdated Show resolved Hide resolved
Pick the GoReleaser build for integration tests by default
Copy link
Member

@akalipetis akalipetis left a comment

Choose a reason for hiding this comment

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

LGTM

@akalipetis akalipetis merged commit f80a4d9 into main Nov 7, 2024
1 check passed
@akalipetis akalipetis deleted the use-golang-tests branch November 7, 2024 12:27
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.

2 participants