Repository: kubernetes/klog
Pull request #113 (2020-02-13 10:27:42)

Modify setVState method to use receiver identifier



<!--  Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, read our contributor guidelines https://git.k8s.io/community/contributors/guide/pull-requests.md#the-pull-request-submit-process and developer guide https://git.k8s.io/community/contributors/devel/development.md#development-guide
2. If you want *faster* PR reviews, read how: https://git.k8s.io/community/contributors/guide/pull-requests.md#best-practices-for-faster-reviews
3. Follow the instructions for writing a release note: https://git.k8s.io/community/contributors/guide/release-notes.md
4. If the PR is unfinished, see how to mark it: https://git.k8s.io/community/contributors/guide/pull-requests.md#marking-unfinished-pull-requests
5. If this PR changes image versions, please title this PR "Bump <image name> from x.x.x to y.y.y."
-->

**What this PR does / why we need it**:
This commit modifies `setVState` method of `loggingT` to use receiver identifier.
Before this commit, this method doesn't use receiver but uses top-level variable `logging`.

**Which issue(s) this PR fixes** :
**None**

**Special notes for your reviewer**:
None

_Please confirm that if this PR changes any image versions, then that's the sole change this PR makes._

**Release note**:
<!--  Write your release note:
1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required".
2. If no release note is required, just write "NONE".
-->
```release-note

```

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes/klog/pull/113#" title="Author self-approved">vanou</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **directxman12**
You can assign the PR to them by writing `/assign @directxman12` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes%2Fklog).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/kubernetes/klog/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["directxman12"]} -->
vanou: Thanks @neolit123 for review. I added test function.

If you have a time, please review them & give comment.
neolit123: please have a look at the CI failures:
https://travis-ci.org/kubernetes/klog/builds/650417325?utm_source=github_status&utm_medium=notification
vanou: @neolit123 
Thank you for kind suggestion. I overlooked it.
And now I fix it.

Patch:
diff --git a/klog.go b/klog.go
index 3511d4a..c334b14 100644
--- a/klog.go
+++ b/klog.go
@@ -520,20 +520,20 @@ var logging loggingT
 // l.mu is held.
 func (l *loggingT) setVState(verbosity Level, filter []modulePat, setFilter bool) {
 	// Turn verbosity off so V will not fire while we are in transition.
-	logging.verbosity.set(0)
+	l.verbosity.set(0)
 	// Ditto for filter length.
-	atomic.StoreInt32(&logging.filterLength, 0)
+	atomic.StoreInt32(&l.filterLength, 0)
 
 	// Set the new filters and wipe the pc->Level map if the filter has changed.
 	if setFilter {
-		logging.vmodule.filter = filter
-		logging.vmap = make(map[uintptr]Level)
+		l.vmodule.filter = filter
+		l.vmap = make(map[uintptr]Level)
 	}
 
 	// Things are consistent now, so enable filtering and verbosity.
 	// They are enabled in order opposite to that in V.
-	atomic.StoreInt32(&logging.filterLength, int32(len(filter)))
-	logging.verbosity.set(verbosity)
+	atomic.StoreInt32(&l.filterLength, int32(len(filter)))
+	l.verbosity.set(verbosity)
 }
 
 // getBuffer returns a new, ready-to-use buffer.


Test patch:
diff --git a/klog_test.go b/klog_test.go
index b6d4218..255cf25 100644
--- a/klog_test.go
+++ b/klog_test.go
@@ -875,3 +875,66 @@ func TestKvListFormat(t *testing.T) {
 		}
 	}
 }
+
+func createTestValueOfLoggingT() *loggingT {
+	l := new(loggingT)
+	l.toStderr = true
+	l.alsoToStderr = false
+	l.stderrThreshold = errorLog
+	l.verbosity = Level(0)
+	l.skipHeaders = false
+	l.skipLogHeaders = false
+	l.addDirHeader = false
+	return l
+}
+
+func createTestValueOfModulePat(p string, li bool, le Level) modulePat {
+	m := modulePat{}
+	m.pattern = p
+	m.literal = li
+	m.level = le
+	return m
+}
+
+func compareModuleSpec(a, b moduleSpec) bool {
+	if len(a.filter) != len(b.filter) {
+		return false
+	}
+
+	for i := 0; i < len(a.filter); i++ {
+		if a.filter[i] != b.filter[i] {
+			return false
+		}
+	}
+
+	return true
+}
+
+func TestSetVState(t *testing.T) {
+	//Target loggingT value
+	want := createTestValueOfLoggingT()
+	want.verbosity = Level(3)
+	want.vmodule.filter = []modulePat{
+		createTestValueOfModulePat("recordio", true, Level(2)),
+		createTestValueOfModulePat("file", true, Level(1)),
+		createTestValueOfModulePat("gfs*", false, Level(3)),
+		createTestValueOfModulePat("gopher*", false, Level(3)),
+	}
+	want.filterLength = 4
+
+	//loggingT value to which test is run
+	target := createTestValueOfLoggingT()
+
+	tf := []modulePat{
+		createTestValueOfModulePat("recordio", true, Level(2)),
+		createTestValueOfModulePat("file", true, Level(1)),
+		createTestValueOfModulePat("gfs*", false, Level(3)),
+		createTestValueOfModulePat("gopher*", false, Level(3)),
+	}
+
+	target.setVState(Level(3), tf, true)
+
+	if want.verbosity != target.verbosity || !compareModuleSpec(want.vmodule, target.vmodule) || want.filterLength != target.filterLength {
+		t.Errorf("setVState method doesn't configure loggingT values' verbosity, vmodule or filterLength:\nwant:\n\tverbosity:\t%v\n\tvmodule:\t%v\n\tfilterLength:\t%v\ngot:\n\tverbosity:\t%v\n\tvmodule:\t%v\n\tfilterLength:\t%v", want.verbosity, want.vmodule, want.filterLength, target.verbosity, target.vmodule, target.filterLength)
+	}
+}