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

trying out running against localhost for test #260

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

michaelneale
Copy link

We had an issue where prod was down and build was failing - not sure if that was the intention of these client tests, but in this case it runs against local.

An experiment at this stage

@michaelneale michaelneale marked this pull request as draft July 25, 2024 08:52
@michaelneale michaelneale removed the request for review from decentralgabe July 25, 2024 08:53
@codecov-commenter
Copy link

codecov-commenter commented Jul 25, 2024

Codecov Report

Attention: Patch coverage is 0% with 25 lines in your changes missing coverage. Please review.

Project coverage is 51.21%. Comparing base (b826172) to head (0b8ef0b).

Files Patch % Lines
impl/concurrencytest/main.go 0.00% 24 Missing ⚠️
impl/integrationtest/main.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #260      +/-   ##
==========================================
+ Coverage   51.14%   51.21%   +0.07%     
==========================================
  Files          31       31              
  Lines        2710     2706       -4     
==========================================
  Hits         1386     1386              
+ Misses       1183     1179       -4     
  Partials      141      141              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@decentralgabe
Copy link
Member

@michaelneale I believe the failing test is here https://github.com/TBD54566975/did-dht/blob/main/impl/internal/did/client_test.go#L15

the test should be updated to use a mock server

and then we should make sure of the integration/concurrency tests (which AFAIK aren't used at all right now) to do something like what you have here. ideally spin up a docker container, hit the endpoints, etc.

@michaelneale
Copy link
Author

@decentralgabe so you prefer a mock server over pointing it to localhost? (so we just test client isolated ideally?)

@decentralgabe
Copy link
Member

@michaelneale I'm most concerned with maintaining test functionality in both local and CI environments

if localhost can work for both - no problem

@michaelneale
Copy link
Author

@decentralgabe please take a look - I was able to run the mage tests locally, but if you want to try and see if it works for you

@michaelneale
Copy link
Author

well dang, that didn't work.

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.

3 participants