Adding unit test cases for init.go and convert.go of AMF #177
Conversation
adding test case for convert.go file
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive unit test coverage for two AMF service components: the initialization logic in pkg/service/init.go and utility conversion functions in internal/util/convert.go. The tests use the testify framework for assertions and include both positive and negative test cases to validate error handling.
- Adds 11 test functions covering AMF application lifecycle methods (logging, context management, initialization)
- Adds 5 test functions covering SNSSAI, AMF ID, PLMN ID, and TAC conversion utilities
- Introduces mock implementations to facilitate isolated unit testing
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 11 comments.
| File | Description |
|---|---|
| pkg/service/init_test.go | Adds unit tests for AmfApp initialization, configuration setters (log level, log enable, report caller), context management, and shutdown event handling. Includes mock server implementation for testing. |
| internal/util/convert_test.go | Adds unit tests for network identifier conversion functions including SNSSAI hex-to-model conversions, AMF ID separation, PLMN ID parsing, and TAC configuration conversion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "github.com/free5gc/amf/pkg/factory" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/mock" | ||
| "github.com/free5gc/openapi/models" |
There was a problem hiding this comment.
Inconsistent indentation detected. This import line uses spaces instead of tabs, which is inconsistent with the other import lines. Go convention and this codebase use tabs for indentation.
| "github.com/free5gc/openapi/models" | |
| "github.com/free5gc/openapi/models" |
| func TestSetLogFilePath(t *testing.T) { | ||
| config := &factory.Config{} | ||
| app := &AmfApp{cfg: config} | ||
|
|
||
| logFilePath := "/tmp/test_amf_log.log" | ||
| f, err := os.Create(logFilePath) | ||
| assert.NoError(t, err) | ||
| defer f.Close() | ||
| defer os.Remove(logFilePath) | ||
|
|
||
| app.SetLogFilePath(f) | ||
| assert.Equal(t, logFilePath, f.Name()) | ||
| } | ||
|
|
There was a problem hiding this comment.
The SetLogFilePath method does not exist in the AmfApp struct or the app.App interface. This test will fail to compile. The app.App interface only defines SetLogEnable, SetLogLevel, and SetReportCaller for logging configuration.
| func TestSetLogFilePath(t *testing.T) { | |
| config := &factory.Config{} | |
| app := &AmfApp{cfg: config} | |
| logFilePath := "/tmp/test_amf_log.log" | |
| f, err := os.Create(logFilePath) | |
| assert.NoError(t, err) | |
| defer f.Close() | |
| defer os.Remove(logFilePath) | |
| app.SetLogFilePath(f) | |
| assert.Equal(t, logFilePath, f.Name()) | |
| } |
| config := &factory.Config{} | ||
| app := &AmfApp{cfg: config} | ||
|
|
||
| app.SetLogEnable(true) |
There was a problem hiding this comment.
This assertion is incorrect. The test calls app.SetLogEnable(true) on line 33 but never sets it to false before the assertion on line 36. The test should verify that SetLogEnable(true) worked first, or this should be testing the false value that was just set.
| app.SetLogEnable(true) | |
| app.SetLogEnable(true) | |
| assert.True(t, app.cfg.GetLogEnable()) |
|
|
||
|
|
There was a problem hiding this comment.
Trailing whitespace on empty line. This should be removed to maintain code cleanliness.
|
|
||
|
|
There was a problem hiding this comment.
Trailing whitespace on empty line. This should be removed to maintain code cleanliness.
| @@ -0,0 +1,167 @@ | |||
| package util | |||
There was a problem hiding this comment.
Trailing whitespace on line. Remove the extra space after 'util' to maintain code cleanliness.
| package util | |
| package util |
| return m.Cfg | ||
| } | ||
|
|
||
| // Wrap the mock inside a dummy real consumer |
There was a problem hiding this comment.
The comment is misleading. The Consumer method doesn't "wrap the mock inside a dummy real consumer" - it actually creates and returns a real Consumer instance. This could be confusing for future maintainers.
| // Wrap the mock inside a dummy real consumer | |
| // Creates and returns a real consumer.Consumer instance for use in tests |
| func TestNewApp(t *testing.T) { | ||
|
|
||
| tmpLog := "/tmp/amf_test.log" | ||
| _ = os.WriteFile(tmpLog, []byte{}, 0644) | ||
| defer os.Remove(tmpLog) | ||
|
|
||
| cfg := &factory.Config{ | ||
| Configuration: &factory.Configuration{ | ||
| ServiceNameList: []string{ | ||
| string(models.ServiceName_NAMF_COMM), | ||
| string(models.ServiceName_NAMF_EVTS), | ||
| string(models.ServiceName_NAMF_MT), | ||
| string(models.ServiceName_NAMF_LOC), | ||
| string(models.ServiceName_NAMF_OAM), | ||
| }, | ||
| }, | ||
| } | ||
| factory.AmfConfig = cfg | ||
| app, err := NewApp(context.Background(), cfg, "") | ||
| assert.NoError(t, err) | ||
| assert.NotNil(t, app) | ||
| } |
There was a problem hiding this comment.
The AMF global variable is being set in NewApp (line 91 of init.go), but this test doesn't verify that behavior. Consider adding an assertion to verify that the global AMF variable is properly set after NewApp is called.
|
|
||
| result := TACConfigToModels(input) | ||
|
|
||
| // When ParseUint fails, it logs and returns empty hex string |
There was a problem hiding this comment.
The comment incorrectly describes the expected behavior. When ParseUint fails with a non-numeric string, the function logs the error but tmp remains 0, resulting in "000000" (6 zeros), not an empty hex string as the comment states.
| // When ParseUint fails, it logs and returns empty hex string | |
| // When ParseUint fails, it logs the error and returns "000000" (since tmp is 0 if err != nil) |
| // In package consumer | ||
| type DummySCTPListener struct{} | ||
|
|
||
| func (d *DummySCTPListener) Close() error { | ||
| // Pretend to close without doing anything | ||
| return nil | ||
| } |
There was a problem hiding this comment.
This comment is misleading. The comment says "In package consumer" but this code is in the service package's test file. Additionally, DummySCTPListener is defined but never used in any test.
| // In package consumer | |
| type DummySCTPListener struct{} | |
| func (d *DummySCTPListener) Close() error { | |
| // Pretend to close without doing anything | |
| return nil | |
| } |
1 participant
test: add init_test.go