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

test: Allow integration tests to run against any accessible cheqd resolver #247

Merged
merged 1 commit into from
Feb 28, 2024

Conversation

jcourt562
Copy link
Contributor

This is a very mechanical/repetitive, although large change to the integration test suite.

Problem Addressed:
The integration tests hardcode the host portion of all test Cheqd DID URLs to be localhost:8080. Whilst this works when running on a developers local machine and on GitHub it doesn't work for Gitlab pipelines which need a docker-in-docker container addressed as 'docker:8080'. Additionally with this set of changes it will be possible to run the integration tests against any publicly accessible/deployed cheqd resolver which could be useful in diagnosing field issues/anomalies.

Solution submitted
This change introduces an integration testing environment string variable called SUT_HOST_ADDRESS.
If the environment variable is set then the SUTHost test constant is set to that value in tests/constants/constants.go. If the environment variable is NOT set then the default behaviour is maintained with SUTHost being set to localhost:8080.
All current integration tests were changed to use SUTHost when creating DidURL using fmt.Sprintf() in place of the hardcoded localhost:8080

@jcourt562 jcourt562 changed the title Allow integration tests to run against any accessible cheqd resolver test: Allow integration tests to run against any accessible cheqd resolver Feb 26, 2024
Copy link
Contributor

@lampkin-diet lampkin-diet left a comment

Choose a reason for hiding this comment

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

Thanks for your time and improvements! The overall look fine to me except variable name :)

tests/constants/common.go Show resolved Hide resolved
@lampkin-diet lampkin-diet changed the base branch from main to develop February 26, 2024 17:25
Copy link
Contributor

@ankurdotb ankurdotb left a comment

Choose a reason for hiding this comment

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

@lampkin-diet Can you please redirect this PR to a different branch that targets develop and just make the changes necessary? That way, our workflows will actually execute: it currently doesn't because it's coming from a fork.

  1. Rename the variable to make it more obvious.
  2. Can we make it default to localhost:8080, and if there's an environment variable provided, use the value provided by the environment variable? That way, there's always a valid value for this which should cover people who aren't specifically providing a value, and those who do can override the default.

@ankurdotb ankurdotb changed the base branch from develop to configurable-tests February 28, 2024 12:03
@ankurdotb ankurdotb changed the base branch from configurable-tests to develop February 28, 2024 12:07
@ankurdotb ankurdotb merged commit 930ed98 into cheqd:develop Feb 28, 2024
5 of 6 checks passed
@jcourt562 jcourt562 deleted the configurable-sut-host branch March 4, 2024 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants