Skip to content

Add comprehensive unit tests for lib package#303

Draft
Copilot wants to merge 7 commits intomasterfrom
copilot/write-unit-tests-for-lib-package
Draft

Add comprehensive unit tests for lib package#303
Copilot wants to merge 7 commits intomasterfrom
copilot/write-unit-tests-for-lib-package

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 6, 2026

  • Analyze all files in lib package to understand functions/methods to test
  • Create lib/lib_test.go - tests for constants, types, IgnoreIPv4/IgnoreIPv6 functions
  • Create lib/error_test.go - tests for error variables
  • Create lib/entry_test.go - tests for Entry (NewEntry, AddPrefix, RemovePrefix, processPrefix, MarshalPrefix, MarshalIPRange, MarshalText, buildIPSet, GetIPv4Set, GetIPv6Set)
  • Create lib/container_test.go - tests for Container (NewContainer, GetEntry, Add, Remove, Loop, Len, Lookup)
  • Create lib/common_test.go - tests for GetIgnoreIPType, GetRemoteURLContent, GetRemoteURLReader, WantedListExtended.UnmarshalJSON
  • Create lib/config_test.go - tests for RegisterInputConfigCreator, RegisterOutputConfigCreator, createInputConfig, createOutputConfig, inputConvConfig.UnmarshalJSON, outputConvConfig.UnmarshalJSON
  • Create lib/converter_test.go - tests for RegisterInputConverter, RegisterOutputConverter, ListInputConverter, ListOutputConverter
  • Create lib/instance_test.go - tests for NewInstance, InitConfig, InitConfigFromBytes, AddInput, AddOutput, ResetInput, ResetOutput, RunInput, RunOutput, Run
  • Verify coverage at 94.1% (remaining 5.9% is unreachable defensive code)
  • Add CIDR prefix assertions to TestContainerAddMergeIgnoreIPv4 and TestContainerAddMergeIgnoreIPv6
  • Add CIDR prefix assertions to TestContainerRemoveCaseRemovePrefix, TestContainerRemoveCaseRemovePrefixIgnoreIPv4, and TestContainerRemoveCaseRemovePrefixIgnoreIPv6
  • Rename and reorder TestContainerLookupIPv6ErrorEntryMissingIPSet
  • Add CIDR prefix assertions to TestContainerRemoveCaseRemovePrefixNoBuilderOnExisting, TestContainerRemoveCaseRemovePrefixNoIPv6BuilderIgnoreIPv4, TestContainerRemoveCaseRemovePrefixNoIPv4BuilderIgnoreIPv6, TestContainerRemoveCaseRemovePrefixNoBuilderDefault, and TestContainerRemoveCaseRemovePrefixOnlyIPv4NoIPv6Builder
  • Verify exact output strings in TestListInputConverter and TestListOutputConverter
  • Run code review and address feedback
  • Run CodeQL checker

🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

Co-authored-by: Loyalsoldier <10487845+Loyalsoldier@users.noreply.github.com>
Copilot AI changed the title [WIP] Add comprehensive unit tests for lib package with 100% coverage Add comprehensive unit tests for lib package Mar 6, 2026
Comment on lines +210 to +258
func TestContainerAddMergeIgnoreIPv4(t *testing.T) {
c := NewContainer()

// First add normally
entry1 := NewEntry("us")
if err := entry1.AddPrefix("1.0.0.0/24"); err != nil {
t.Fatal(err)
}
if err := c.Add(entry1); err != nil {
t.Fatal(err)
}

// Add with IgnoreIPv4 to existing entry
entry2 := NewEntry("us")
if err := entry2.AddPrefix("2.0.0.0/24"); err != nil {
t.Fatal(err)
}
if err := entry2.AddPrefix("2001:db8::/32"); err != nil {
t.Fatal(err)
}
if err := c.Add(entry2, IgnoreIPv4); err != nil {
t.Fatal(err)
}
}

func TestContainerAddMergeIgnoreIPv6(t *testing.T) {
c := NewContainer()

// First add normally
entry1 := NewEntry("us")
if err := entry1.AddPrefix("2001:db8::/32"); err != nil {
t.Fatal(err)
}
if err := c.Add(entry1); err != nil {
t.Fatal(err)
}

// Add with IgnoreIPv6 to existing entry
entry2 := NewEntry("us")
if err := entry2.AddPrefix("1.0.0.0/24"); err != nil {
t.Fatal(err)
}
if err := entry2.AddPrefix("2001:db9::/32"); err != nil {
t.Fatal(err)
}
if err := c.Add(entry2, IgnoreIPv6); err != nil {
t.Fatal(err)
}
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot These two tests miss checking resulting CIDR prefixes at the end, please fix them.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in d0d5153. Both tests now verify the resulting CIDR prefixes — TestContainerAddMergeIgnoreIPv4 checks that the original IPv4 (1.0.0.0/24) is preserved and only IPv6 (2001:db8::/32) is merged, and TestContainerAddMergeIgnoreIPv6 checks that the original IPv6 (2001:db8::/32) is preserved and only IPv4 (1.0.0.0/24) is merged.

Co-authored-by: Loyalsoldier <10487845+Loyalsoldier@users.noreply.github.com>
Copy link
Copy Markdown
Author

Copilot AI commented Mar 8, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • invalid-host-that-does-not-exist.example.com
    • Triggering command: /tmp/go-build798624579/b001/lib.test /tmp/go-build798624579/b001/lib.test -test.paniconexit0 -test.gocoverdir=/tmp/go-build798624579/b001/gocoverdir -test.timeout=10m0s -test.count=1 -test.coverprofile=/tmp/go-build798624579/b001/_cover_.out (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Co-authored-by: Loyalsoldier <10487845+Loyalsoldier@users.noreply.github.com>
Copy link
Copy Markdown
Author

Copilot AI commented Mar 8, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • invalid-host-that-does-not-exist.example.com
    • Triggering command: /tmp/go-build2553015557/b001/lib.test /tmp/go-build2553015557/b001/lib.test -test.paniconexit0 -test.timeout=10m0s -test.count=1 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

…upIPv6ErrorEntryMissingIPSet and reorder

Co-authored-by: Loyalsoldier <10487845+Loyalsoldier@users.noreply.github.com>
@Loyalsoldier
Copy link
Copy Markdown
Owner

@copilot Test TestContainerRemoveCaseRemovePrefixNoBuilderOnExisting, TestContainerRemoveCaseRemovePrefixNoIPv6BuilderIgnoreIPv4, TestContainerRemoveCaseRemovePrefixNoIPv4BuilderIgnoreIPv6, TestContainerRemoveCaseRemovePrefixNoBuilderDefault, TestContainerRemoveCaseRemovePrefixOnlyIPv4NoIPv6Builder miss checking resulting CIDR prefixes at the end, please fix them.

Co-authored-by: Loyalsoldier <10487845+Loyalsoldier@users.noreply.github.com>
Copy link
Copy Markdown
Author

Copilot AI commented Mar 8, 2026

@copilot Test TestContainerRemoveCaseRemovePrefixNoBuilderOnExisting, TestContainerRemoveCaseRemovePrefixNoIPv6BuilderIgnoreIPv4, `TestContainerRemoveCaseRemovePrefixNoIPv4BuilderI...

Fixed in 19c8c8d. All five tests now verify resulting CIDR prefixes:

  • TestContainerRemoveCaseRemovePrefixNoBuilderOnExisting — confirms 1.0.0.0/24 remains, 0 IPv6 prefixes
  • TestContainerRemoveCaseRemovePrefixNoIPv6BuilderIgnoreIPv4 — confirms 1.0.0.0/24 remains (ignored), 0 IPv6 prefixes
  • TestContainerRemoveCaseRemovePrefixNoIPv4BuilderIgnoreIPv6 — confirms 2001:db8::/32 remains (ignored), 0 IPv4 prefixes
  • TestContainerRemoveCaseRemovePrefixNoBuilderDefault — confirms 0 IPv4 prefixes and remaining IPv6 prefixes after removing /48 from /32
  • TestContainerRemoveCaseRemovePrefixOnlyIPv4NoIPv6Builder — confirms 1.0.0.128/25 remains after removing /25 from /24, 0 IPv6 prefixes

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • invalid-host-that-does-not-exist.example.com
    • Triggering command: /tmp/go-build31204441/b001/lib.test /tmp/go-build31204441/b001/lib.test -test.testlogfile=/tmp/go-build31204441/b001/testlog.txt -test.paniconexit0 -test.timeout=10m0s -test.v=true (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Comment on lines +45 to +97
func TestListInputConverter(t *testing.T) {
origMap := inputConverterMap
inputConverterMap = make(map[string]InputConverter)
defer func() { inputConverterMap = origMap }()

mock := &mockInputConverter{typeName: "test-ic", action: ActionAdd, description: "Test Input"}
RegisterInputConverter("test-ic", mock)

// Capture stdout
old := os.Stdout
r, w, _ := os.Pipe()
os.Stdout = w

ListInputConverter()

w.Close()
os.Stdout = old

var buf bytes.Buffer
buf.ReadFrom(r)
output := buf.String()

if len(output) == 0 {
t.Error("ListInputConverter should produce output")
}
}

func TestListOutputConverter(t *testing.T) {
origMap := outputConverterMap
outputConverterMap = make(map[string]OutputConverter)
defer func() { outputConverterMap = origMap }()

mock := &mockOutputConverter{typeName: "test-oc", action: ActionOutput, description: "Test Output"}
RegisterOutputConverter("test-oc", mock)

// Capture stdout
old := os.Stdout
r, w, _ := os.Pipe()
os.Stdout = w

ListOutputConverter()

w.Close()
os.Stdout = old

var buf bytes.Buffer
buf.ReadFrom(r)
output := buf.String()

if len(output) == 0 {
t.Error("ListOutputConverter should produce output")
}
}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot Please verify the output string.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in 16b530c. Both tests now verify the exact output string — TestListInputConverter checks for "All available input formats:\n - test-ic (Test Input)\n" and TestListOutputConverter checks for "All available output formats:\n - test-oc (Test Output)\n".

…putConverter

Co-authored-by: Loyalsoldier <10487845+Loyalsoldier@users.noreply.github.com>
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