🐛 Make sure that loggers added in NewMultipleLoggers don't fail with 'No Logger Source' (#451)

<!--
Copyright (C) 2020-2022 Arm Limited or its affiliates and Contributors.
All rights reserved.
SPDX-License-Identifier: Apache-2.0
-->
### Description

<!--
Please add any detail or context that would be useful to a reviewer.
-->

Make sure that loggers added in NewMultipleLoggers don't fail with 'No
Logger Source'.

NewMultipleLoggers calls Append to add the loggers, this tries to call
c.setLoggerSource but at this point the base logger hasn't had it's
source set. This change makes sure that the source is set earlier in the
process.

It also adds a test that initialises the loggers with NewMultipleLoggers
as there wasn't a test which is why this problem was missed.

### Test Coverage

<!--
Please put an `x` in the correct box e.g. `[x]` to indicate the testing
coverage of this change.
-->

- [x]  This change is covered by existing or additional automated tests.
- [ ] Manual testing has been performed (and evidence provided) as
automated testing was not feasible.
- [ ] Additional tests are not required for this change (e.g.
documentation update).

Author: joshjennings98

changes/20240424131658.bugfix

@@ -0,0 +1 @@
+:bug: Make sure that loggers added in NewMultipleLoggers don't fail with 'No Logger Source'

utils/logs/multiple_logger.go

@@ -55,11 +55,13 @@ func (c *MultipleLogger) setLoggerSource(source string) error {
 	var err error
 	for i := range c.loggers {
 		err = c.loggers[i].SetLoggerSource(source)
+		if err != nil {
+			return err
+		}
 	}
-	if err == nil {
-		c.loggerSource = source
-	}
-	return err
+
+	c.loggerSource = source
+	return nil
 }
 
 func (c *MultipleLogger) Log(output ...interface{}) {
@@ -123,6 +125,12 @@ func NewMultipleLoggers(loggerSource string, loggersList ...Loggers) (l IMultipl
 		err = commonerrors.ErrNoLoggerSource
 		return
 	}
+	l = &MultipleLoggerWithLoggerSource{}
+	err = l.SetLoggerSource(loggerSource)
+	if err != nil {
+		return
+	}
+
 	list := loggersList
 	if len(list) == 0 {
 		std, err := NewStdLogger(loggerSource)
@@ -131,12 +139,7 @@ func NewMultipleLoggers(loggerSource string, loggersList ...Loggers) (l IMultipl
 		}
 		list = []Loggers{std}
 	}
-	l = &MultipleLoggerWithLoggerSource{}
 	err = l.Append(list...)
-	if err != nil {
-		return
-	}
-	err = l.SetLoggerSource(loggerSource)
 	return
 }
 

utils/logs/multiple_logger_test.go

@@ -37,74 +37,100 @@ func TestCombinedLogger(t *testing.T) {
 }
 
 func TestMultipleLoggers(t *testing.T) {
-	// With default logger
-	loggers, err := NewMultipleLoggers("Test Multiple")
-	require.NoError(t, err)
-	testLog(t, loggers)
+	t.Run("Manually add loggers", func(t *testing.T) {
+		// With default logger
+		loggers, err := NewMultipleLoggers("Test Multiple")
+		require.NoError(t, err)
+		testLog(t, loggers)
 
-	// Adding a file logger to the mix.
-	file, err := filesystem.TempFileInTempDir("test-multiplelog-filelog-*.log")
-	require.NoError(t, err)
+		// Adding a file logger to the mix.
+		file, err := filesystem.TempFileInTempDir("test-multiplelog-filelog-*.log")
+		require.NoError(t, err)
 
-	err = file.Close()
-	require.NoError(t, err)
+		err = file.Close()
+		require.NoError(t, err)
 
-	defer func() { _ = filesystem.Rm(file.Name()) }()
+		defer func() { _ = filesystem.Rm(file.Name()) }()
 
-	empty, err := filesystem.IsEmpty(file.Name())
-	require.NoError(t, err)
-	assert.True(t, empty)
+		empty, err := filesystem.IsEmpty(file.Name())
+		require.NoError(t, err)
+		assert.True(t, empty)
 
-	fl, err := NewFileLogger(file.Name(), "Test")
-	require.NoError(t, err)
+		fl, err := NewFileLogger(file.Name(), "Test")
+		require.NoError(t, err)
 
-	require.NoError(t, loggers.Append(fl))
+		require.NoError(t, loggers.Append(fl))
 
-	nl, err := NewNoopLogger("Test2")
-	require.NoError(t, err)
+		nl, err := NewNoopLogger("Test2")
+		require.NoError(t, err)
 
-	// Adding various loggers
-	require.NoError(t, loggers.Append(fl, nl))
+		// Adding various loggers
+		require.NoError(t, loggers.Append(fl, nl))
 
-	testLog(t, loggers)
+		testLog(t, loggers)
 
-	empty, err = filesystem.IsEmpty(file.Name())
-	require.NoError(t, err)
-	assert.False(t, empty)
+		empty, err = filesystem.IsEmpty(file.Name())
+		require.NoError(t, err)
+		assert.False(t, empty)
 
-	err = filesystem.Rm(file.Name())
-	require.NoError(t, err)
+		err = filesystem.Rm(file.Name())
+		require.NoError(t, err)
 
-	// Concurrency test multiple loggers
+		// Concurrency test multiple loggers
 
-	stdLogger, err := NewStdLogger("Test std logger")
-	require.NoError(t, err)
+		stdLogger, err := NewStdLogger("Test std logger")
+		require.NoError(t, err)
 
-	stringLogger, err := NewPlainStringLogger()
-	if err != nil {
-		return
-	}
-	mLoggers, err := NewCombinedLoggers(stdLogger, stringLogger)
-	if err != nil {
-		return
-	}
-
-	wg := sync.WaitGroup{}
-	wg.Add(2)
-	go func() {
-		defer wg.Done()
-		for i := 0; i < 100; i++ {
-			mLoggers.Log(fmt.Sprintf("Test output %v", i))
+		stringLogger, err := NewPlainStringLogger()
+		if err != nil {
+			return
 		}
-	}()
-	go func() {
-		defer wg.Done()
-		for i := 0; i < 100; i++ {
-			mLoggers.LogError(fmt.Sprintf("Test output %v", i))
+		mLoggers, err := NewCombinedLoggers(stdLogger, stringLogger)
+		if err != nil {
+			return
 		}
-	}()
 
-	wg.Wait()
-	err = loggers.Close()
-	require.NoError(t, err)
+		wg := sync.WaitGroup{}
+		wg.Add(2)
+		go func() {
+			defer wg.Done()
+			for i := 0; i < 100; i++ {
+				mLoggers.Log(fmt.Sprintf("Test output %v", i))
+			}
+		}()
+		go func() {
+			defer wg.Done()
+			for i := 0; i < 100; i++ {
+				mLoggers.LogError(fmt.Sprintf("Test output %v", i))
+			}
+		}()
+
+		wg.Wait()
+		err = loggers.Close()
+		require.NoError(t, err)
+	})
+
+	t.Run("Add loggers at start", func(t *testing.T) {
+		file, err := filesystem.TempFileInTempDir("test-multiplelog-filelog-*.log")
+		require.NoError(t, err)
+
+		err = file.Close()
+		require.NoError(t, err)
+
+		defer func() { _ = filesystem.Rm(file.Name()) }()
+
+		empty, err := filesystem.IsEmpty(file.Name())
+		require.NoError(t, err)
+		assert.True(t, empty)
+
+		fl, err := NewFileLogger(file.Name(), "Test")
+		require.NoError(t, err)
+
+		nl, err := NewNoopLogger("Test2")
+		require.NoError(t, err)
+
+		loggers, err := NewMultipleLoggers("Test Multiple", fl, nl)
+		require.NoError(t, err)
+		testLog(t, loggers)
+	})
 }