Skip to content

[P2P] test: URL conversion with mockdns#586

Merged
bryanchriswhite merged 5 commits intomainfrom
test/mockdns
Mar 15, 2023
Merged

[P2P] test: URL conversion with mockdns#586
bryanchriswhite merged 5 commits intomainfrom
test/mockdns

Conversation

@bryanchriswhite
Copy link
Copy Markdown
Collaborator

@bryanchriswhite bryanchriswhite commented Mar 15, 2023

Description

This was a TECHDEBT item that became trivial to do as work progressed on another task which also required use of mockdns (#576).

Issue

Not related to any specific issue.

Type of change

Please mark the relevant option(s):

  • New feature, functionality or library
  • Bug fix
  • Code health or cleanup
  • Major breaking change
  • Documentation
  • Other

List of changes

  • Adds mockdns as a test dependency
  • Mocks DNS resolution in url_conversion_test.go
  • Adds regression tests to url_conversion_test.go for single- and multi-record DNS responses

Testing

  • make develop_test
  • LocalNet w/ all of the steps outlined in the README

Required Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have tested my changes using the available tooling
  • I have updated the corresponding CHANGELOG

If Applicable Checklist

  • I have updated the corresponding README(s); local and/or global
  • I have added tests that prove my fix is effective or that my feature works
  • I have added, or updated, mermaid.js diagrams in the corresponding README(s)
  • I have added, or updated, documentation and mermaid.js diagrams in shared/docs/* if I updated shared/*README(s)

@bryanchriswhite bryanchriswhite added the p2p P2P specific changes label Mar 15, 2023
@bryanchriswhite bryanchriswhite self-assigned this Mar 15, 2023
@bryanchriswhite bryanchriswhite changed the title [P2P] test: url_conversion with mockdns [P2P] test: URL conversion with mockdns Mar 15, 2023
@bryanchriswhite bryanchriswhite marked this pull request as ready for review March 15, 2023 09:54
quadARecord dnsRecordType = "AAAA"
)

func prepareDNSResolverMock(t *testing.T, zones map[string]mockdns.Zone) (done func()) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! 😍

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💯

Copy link
Copy Markdown
Contributor

@deblasis deblasis left a comment

Choose a reason for hiding this comment

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

Nice one @bryanchriswhite !

I wonder if the mockDNS change could also solve the issue #584

At a glance, it sounds like it might be a quick win. Wdyt?

@bryanchriswhite
Copy link
Copy Markdown
Collaborator Author

@deblasis Thanks for calling out that issue, I hadn't see it yet. 🙌 I think it would be best to tackle that one in a separate PR. I don't think it's as straightforward as adding another usage of mockdns.

Copy link
Copy Markdown
Contributor

@deblasis deblasis left a comment

Choose a reason for hiding this comment

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

LGTM!

@bryanchriswhite bryanchriswhite merged commit 133405c into main Mar 15, 2023
bryanchriswhite added a commit that referenced this pull request Mar 16, 2023
* pokt/main:
  [P2P] test: URL conversion with mockdns (#586)
  Create devlog4.md
  Update devlog3.md to remove demo
  Create devlog3.md
  [CLI] add private keys embed to debug cli build (#578)
  [Documentation] Fix SLIP Master Key and Child Key diagrams to be in the correct order (#573)
  add get secrets perms to cluster-manager-account service account (#572)
@Olshansk Olshansk deleted the test/mockdns branch March 16, 2023 20:55
@@ -173,33 +182,134 @@ func TestServiceURLFromLibp2pMultiaddr_Error(t *testing.T) {

// TECHDEBT: add helpers for crating and/or using a "test resolver" which can
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Should we remove this comment?

quadARecord dnsRecordType = "AAAA"
)

func prepareDNSResolverMock(t *testing.T, zones map[string]mockdns.Zone) (done func()) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p2p P2P specific changes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants