more tests by yegor256 · Pull Request #144 · cqfn/refrax

yegor256 · 2025-12-16T06:40:55Z

closes #138

yegor256 · 2025-12-16T06:41:39Z

Copilot

Pull request overview

This PR adds comprehensive test coverage across multiple packages in the refrax codebase to address issue #138. The tests cover server initialization, lifecycle management, agent behavior, and protocol data structures.

Key Changes

Added test files for the reviewer, fixer, and facilitator packages covering server and agent functionality
Added test files for the protocol package covering message parts and model structures
Implemented mock structures and helper functions to facilitate isolated unit testing

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
internal/reviewer/server_test.go	Tests for reviewer server lifecycle, message handling, and agent card creation
internal/reviewer/agent_test.go	Tests for suggestion parsing and review functionality
internal/protocol/part_test.go	Tests for message part creation, metadata handling, and JSON marshaling
internal/protocol/model_test.go	Tests for agent card builder pattern and data structures
internal/fixer/server_test.go	Tests for fixer server lifecycle, code cleaning, and message handling
internal/facilitator/server_test.go	Tests for facilitator server lifecycle and refactoring coordination
internal/facilitator/agent_test.go	Tests for refactoring logic, class matching, and critique handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-16T06:44:58Z

+func secureRandomInt(limit int) int {
+	n, _ := rand.Int(rand.Reader, big.NewInt(int64(limit)))


The error returned by rand.Int is being silently ignored. This could lead to undefined behavior if random number generation fails. Consider handling the error appropriately or documenting why it's safe to ignore.

Suggested change

func secureRandomInt(limit int) int {

n, _ := rand.Int(rand.Reader, big.NewInt(int64(limit)))

func secureRandomInt(t *testing.T, limit int) int {

n, err := rand.Int(rand.Reader, big.NewInt(int64(limit)))

if err != nil {

t.Fatalf("failed to generate secure random int: %v", err)

}

Copilot · 2025-12-16T06:44:59Z

+)
+
+func randomPort() int {
+	n, _ := rand.Int(rand.Reader, big.NewInt(10000))


The error returned by rand.Int is being silently ignored. This could lead to undefined behavior if random number generation fails. Consider handling the error appropriately or documenting why it's safe to ignore.

Suggested change

n, _ := rand.Int(rand.Reader, big.NewInt(10000))

n, err := rand.Int(rand.Reader, big.NewInt(10000))

if err != nil {

panic("failed to generate random port: " + err.Error())

}

Copilot · 2025-12-16T06:44:59Z

+	reeady := make(chan bool, 1)
+	reeady <- true
+	close(reeady)
+	m.ready = reeady


Misspelled variable name 'reeady' should be 'ready'.

Suggested change

reeady := make(chan bool, 1)

reeady <- true

close(reeady)

m.ready = reeady

ready := make(chan bool, 1)

ready <- true

close(ready)

m.ready = ready

Copilot · 2025-12-16T06:44:59Z

+	reeady := make(chan bool, 1)
+	reeady <- true
+	close(reeady)
+	m.ready = reeady


Misspelled variable name 'reeady' should be 'ready'.

Suggested change

reeady := make(chan bool, 1)

reeady <- true

close(reeady)

m.ready = reeady

ready := make(chan bool, 1)

ready <- true

close(ready)

m.ready = ready

Copilot · 2025-12-16T06:45:00Z

+	reeady := make(chan bool, 1)
+	reeady <- true
+	close(reeady)
+	m.ready = reeady


Misspelled variable name 'reeady' should be 'ready'.

Suggested change

reeady := make(chan bool, 1)

reeady <- true

close(reeady)

m.ready = reeady

ready := make(chan bool, 1)

ready <- true

close(ready)

m.ready = ready

Copilot · 2025-12-16T06:45:00Z

+func randomPort() int {
+	n, _ := rand.Int(rand.Reader, big.NewInt(10000))


The error returned by rand.Int is being silently ignored. This could lead to undefined behavior if random number generation fails. Consider handling the error appropriately or documenting why it's safe to ignore.

Suggested change

func randomPort() int {

n, _ := rand.Int(rand.Reader, big.NewInt(10000))

func randomPort(t *testing.T) int {

n, err := rand.Int(rand.Reader, big.NewInt(10000))

require.NoError(t, err)

Copilot · 2025-12-16T06:45:00Z

+func randomPort() int {
+	n, _ := rand.Int(rand.Reader, big.NewInt(10000))
+	return int(n.Int64()) + 20000
+}
+
+type mock struct {
+	started bool
+	closed  bool
+	handler protocol.MsgHandler
+	ready   chan bool
+}
+
+func (m *mock) ListenAndServe() error {
+	if m.started {
+		return errors.New("server already started")
+	}
+	reeady := make(chan bool, 1)
+	reeady <- true
+	close(reeady)
+	m.ready = reeady
+	m.started = true
+	return nil
+}
+
+func (m *mock) Shutdown() error {
+	if !m.started {
+		return errors.New("server not started")
+	}
+	if m.closed {
+		return errors.New("server already closed")
+	}
+	m.closed = true
+	return nil
+}
+
+func (m *mock) Handler(_ protocol.Handler) {
+}
+
+func (m *mock) MsgHandler(handler protocol.MsgHandler) {
+	m.handler = handler
+}
+
+func (m *mock) Ready() <-chan bool {
+	return m.ready
+}


The randomPort function and mock struct are duplicated across internal/reviewer/server_test.go, internal/fixer/server_test.go, and internal/facilitator/server_test.go. Consider extracting these test utilities into a shared testing package to improve maintainability and reduce code duplication.

Copilot · 2025-12-16T06:45:01Z

+	card := NewAgentCard().
+		WithName("Cömplëtë Ägënt").
+		WithDescription("Ä cömplëtë ägënt cärd").
+		WithURL("http://löcälhöst:" + string(rune(port))).


Incorrect type conversion: attempting to convert an int to a rune and then to a string will produce a single character (the rune representation of the number), not the string representation of the port number. Use strconv.Itoa(port) instead.

Copilot · 2025-12-16T06:45:01Z

+)
+
+func randomPort() int {
+	n, _ := rand.Int(rand.Reader, big.NewInt(10000))


The error returned by rand.Int is being silently ignored. This could lead to undefined behavior if random number generation fails. Consider handling the error appropriately or documenting why it's safe to ignore.

Suggested change

n, _ := rand.Int(rand.Reader, big.NewInt(10000))

n, err := rand.Int(rand.Reader, big.NewInt(10000))

if err != nil {

panic("failed to generate random port: " + err.Error())

}

Copilot · 2025-12-16T06:45:01Z

+)
+
+func randomInt(limit int) int {
+	n, _ := rand.Int(rand.Reader, big.NewInt(int64(limit)))


The error returned by rand.Int is being silently ignored. This could lead to undefined behavior if random number generation fails. Consider handling the error appropriately or documenting why it's safe to ignore.

Suggested change

n, _ := rand.Int(rand.Reader, big.NewInt(int64(limit)))

n, err := rand.Int(rand.Reader, big.NewInt(int64(limit)))

if err != nil {

panic("failed to generate random int: " + err.Error())

}

volodya-lombrozo

@yegor256 This PR is too big. I will add the check for such a big PRs:

#145 (similar to EO check).

Could you split this PR on several small ones, please?

more

2d0ab44

Copilot AI review requested due to automatic review settings December 16, 2025 06:40

Copilot started reviewing on behalf of yegor256 December 16, 2025 06:41 View session

yegor256 requested a review from volodya-lombrozo December 16, 2025 06:41

Copilot AI reviewed Dec 16, 2025

View reviewed changes

volodya-lombrozo mentioned this pull request Dec 16, 2025

add PR size check workflow #145

Open

volodya-lombrozo requested changes Dec 16, 2025

View reviewed changes

		func secureRandomInt(limit int) int {
		n, _ := rand.Int(rand.Reader, big.NewInt(int64(limit)))

-func secureRandomInt(limit int) int {
-	n, _ := rand.Int(rand.Reader, big.NewInt(int64(limit)))
+func secureRandomInt(t *testing.T, limit int) int {
+	n, err := rand.Int(rand.Reader, big.NewInt(int64(limit)))
+	if err != nil {
+		t.Fatalf("failed to generate secure random int: %v", err)
+	}

		func randomPort() int {
		n, _ := rand.Int(rand.Reader, big.NewInt(10000))

-func randomPort() int {
-	n, _ := rand.Int(rand.Reader, big.NewInt(10000))
+func randomPort(t *testing.T) int {
+	n, err := rand.Int(rand.Reader, big.NewInt(10000))
+	require.NoError(t, err)

Conversation

yegor256 commented Dec 16, 2025

Uh oh!

yegor256 commented Dec 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

volodya-lombrozo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants