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

Make the integration test not dependent on version #180

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jonasgeiler
Copy link

@jonasgeiler jonasgeiler commented Oct 28, 2024

I was building supercronic locally and ran the integration tests and noticed this little annoyance. I don't think the integration test should check the exact version it expects, just whether the version is unset or not.

I am open for discussion, though.

:shipit:

P.S.: I just submitted the supercronic, supercronic-bin and supercronic-git packages to the AUR and will be maintaining them as long as I can.

@UserNotFound UserNotFound removed the request for review from krallin October 28, 2024 17:05
@UserNotFound
Copy link
Member

The test is seemingly important because it ensure a version number is actually returned. In what scenario for local testing does this cause an issue?

integration/test.bats Outdated Show resolved Hide resolved
@jonasgeiler
Copy link
Author

jonasgeiler commented Oct 30, 2024

@UserNotFound wrote: The test is seemingly important because it ensure a version number is actually returned. In what scenario for local testing does this cause an issue?

While writing the AUR packages I wanted to verify that everything is working by running the integration tests after build. But I seemingly could not get it to pass this version test. I have also not seen a successful GitHub Actions workflow run for the test workflow in a while, so maybe the test is actually outdated?

Edit: I think I just figured out that the integration test is not supposed to be run on a release build, but only on a special test build, built with a VERSION=v1337 environment variable. I completely misunderstood your usage of this integration test. My bad.

Although I do think the integration test should be written to still pass, even when not hardcoding the version to v1337...

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