This is an automated email from the ASF dual-hosted git repository.
uros-b pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new 81b20d969098 [SPARK-58709][CORE][TEST] Use assert(!x) instead of
assert(x === false) in SecurityManagerSuite
81b20d969098 is described below
commit 81b20d9690980dce71e23590ee37b0ae493aaacc
Author: Uros <[email protected]>
AuthorDate: Mon Aug 17 10:19:35 2026 +0000
[SPARK-58709][CORE][TEST] Use assert(!x) instead of assert(x === false) in
SecurityManagerSuite
### What changes were proposed in this pull request?
Rewrites 19 `assert(expr === false)` comparisons in `SecurityManagerSuite`
to `assert(!expr)`.
### Why are the changes needed?
All three receivers return a Scala `Boolean` (`checkUIViewPermissions`,
`checkModifyPermissions`, `aclsEnabled`), so comparing against `false` adds
nothing.
The negated form also produces a better failure message. ScalaTest's
`assert` macro reconstructs the source expression, so a failure reports the
offending call and its argument:
```
assert(!checkUIViewPermissions("user3")) ->
checkUIViewPermissions("user3") was true
assert(checkUIViewPermissions("user3") === false) -> true did not equal
false
```
In a test body with a dozen near-identical permission assertions, the
second message does not identify which one failed.
### Does this PR introduce _any_ user-facing change?
No. Test-only, and the assertions have identical truth values.
### How was this patch tested?
Existing suite. The rewrite is mechanical and every receiver was confirmed
to be a Scala `Boolean` method, so there is no unboxing or null hazard.
The scope is the first six tests, which is where the pattern is densest;
the occurrences in the later tests are left for a follow-up to keep this diff
reviewable. Happy to fold in the rest if preferred.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)
Closes #57927 from uros-b/cleanup-securitymanager-bool-assert.
Authored-by: Uros <[email protected]>
Signed-off-by: Uros Bojanic <[email protected]>
(cherry picked from commit a21b1453a39fd4b88e558d94e1a89641b8870f3c)
Signed-off-by: Uros Bojanic <[email protected]>
---
.../org/apache/spark/SecurityManagerSuite.scala | 38 +++++++++++-----------
1 file changed, 19 insertions(+), 19 deletions(-)
diff --git a/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala
b/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala
index 9db1afe0853a..f4ecc540155d 100644
--- a/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala
+++ b/core/src/test/scala/org/apache/spark/SecurityManagerSuite.scala
@@ -53,7 +53,7 @@ class SecurityManagerSuite extends SparkFunSuite with
ResetSystemProperties {
assert(securityManager.aclsEnabled())
assert(securityManager.checkUIViewPermissions("user1"))
assert(securityManager.checkUIViewPermissions("user2"))
- assert(securityManager.checkUIViewPermissions("user3") === false)
+ assert(!securityManager.checkUIViewPermissions("user3"))
}
test("set security with conf for groups") {
@@ -65,8 +65,8 @@ class SecurityManagerSuite extends SparkFunSuite with
ResetSystemProperties {
// default ShellBasedGroupsMappingProvider is used to resolve user groups
val securityManager = new SecurityManager(conf);
// assuming executing user does not belong to group1,group2
- assert(securityManager.checkUIViewPermissions("user1") === false)
- assert(securityManager.checkUIViewPermissions("user2") === false)
+ assert(!securityManager.checkUIViewPermissions("user1"))
+ assert(!securityManager.checkUIViewPermissions("user2"))
val conf2 = new SparkConf
conf2.set(NETWORK_AUTH_ENABLED, true)
@@ -91,8 +91,8 @@ class SecurityManagerSuite extends SparkFunSuite with
ResetSystemProperties {
val securityManager3 = new SecurityManager(conf3)
// BogusServiceProvider cannot be loaded and an error is logged returning
an empty group set
- assert(securityManager3.checkUIViewPermissions("user1") === false)
- assert(securityManager3.checkUIViewPermissions("user2") === false)
+ assert(!securityManager3.checkUIViewPermissions("user1"))
+ assert(!securityManager3.checkUIViewPermissions("user2"))
}
test("set security with api") {
@@ -102,7 +102,7 @@ class SecurityManagerSuite extends SparkFunSuite with
ResetSystemProperties {
securityManager.setAcls(true)
assert(securityManager.aclsEnabled())
securityManager.setAcls(false)
- assert(securityManager.aclsEnabled() === false)
+ assert(!securityManager.aclsEnabled())
// acls are off so doesn't matter what view acls set to
assert(securityManager.checkUIViewPermissions("user4"))
@@ -110,11 +110,11 @@ class SecurityManagerSuite extends SparkFunSuite with
ResetSystemProperties {
securityManager.setAcls(true)
assert(securityManager.aclsEnabled())
securityManager.setViewAcls(Set[String]("user5"), Seq("user6", "user7"))
- assert(securityManager.checkUIViewPermissions("user1") === false)
+ assert(!securityManager.checkUIViewPermissions("user1"))
assert(securityManager.checkUIViewPermissions("user5"))
assert(securityManager.checkUIViewPermissions("user6"))
assert(securityManager.checkUIViewPermissions("user7"))
- assert(securityManager.checkUIViewPermissions("user8") === false)
+ assert(!securityManager.checkUIViewPermissions("user8"))
assert(securityManager.checkUIViewPermissions(null))
}
@@ -132,8 +132,8 @@ class SecurityManagerSuite extends SparkFunSuite with
ResetSystemProperties {
// change groups so they do not match
securityManager.setViewAclsGroups(Seq("group4", "group5"))
- assert(securityManager.checkUIViewPermissions("user1") === false)
- assert(securityManager.checkUIViewPermissions("user2") === false)
+ assert(!securityManager.checkUIViewPermissions("user1"))
+ assert(!securityManager.checkUIViewPermissions("user2"))
val conf2 = new SparkConf
conf.set(USER_GROUPS_MAPPING, "BogusServiceProvider")
@@ -143,13 +143,13 @@ class SecurityManagerSuite extends SparkFunSuite with
ResetSystemProperties {
securityManager2.setViewAclsGroups(Seq("group1", "group2"))
// group1,group2 do not match because of BogusServiceProvider
- assert(securityManager.checkUIViewPermissions("user1") === false)
- assert(securityManager.checkUIViewPermissions("user2") === false)
+ assert(!securityManager.checkUIViewPermissions("user1"))
+ assert(!securityManager.checkUIViewPermissions("user2"))
// setting viewAclsGroups to empty should still not match because of
BogusServiceProvider
securityManager2.setViewAclsGroups(Nil)
- assert(securityManager.checkUIViewPermissions("user1") === false)
- assert(securityManager.checkUIViewPermissions("user2") === false)
+ assert(!securityManager.checkUIViewPermissions("user1"))
+ assert(!securityManager.checkUIViewPermissions("user2"))
}
test("set security modify acls") {
@@ -160,7 +160,7 @@ class SecurityManagerSuite extends SparkFunSuite with
ResetSystemProperties {
securityManager.setAcls(true)
assert(securityManager.aclsEnabled())
securityManager.setAcls(false)
- assert(securityManager.aclsEnabled() === false)
+ assert(!securityManager.aclsEnabled())
// acls are off so doesn't matter what view acls set to
assert(securityManager.checkModifyPermissions("user4"))
@@ -168,11 +168,11 @@ class SecurityManagerSuite extends SparkFunSuite with
ResetSystemProperties {
securityManager.setAcls(true)
assert(securityManager.aclsEnabled())
securityManager.setModifyAcls(Set("user5"), Seq("user6", "user7"))
- assert(securityManager.checkModifyPermissions("user1") === false)
+ assert(!securityManager.checkModifyPermissions("user1"))
assert(securityManager.checkModifyPermissions("user5"))
assert(securityManager.checkModifyPermissions("user6"))
assert(securityManager.checkModifyPermissions("user7"))
- assert(securityManager.checkModifyPermissions("user8") === false)
+ assert(!securityManager.checkModifyPermissions("user8"))
assert(securityManager.checkModifyPermissions(null))
}
@@ -190,8 +190,8 @@ class SecurityManagerSuite extends SparkFunSuite with
ResetSystemProperties {
// change groups so they do not match
securityManager.setModifyAclsGroups(Seq("group4", "group5"))
- assert(securityManager.checkModifyPermissions("user1") === false)
- assert(securityManager.checkModifyPermissions("user2") === false)
+ assert(!securityManager.checkModifyPermissions("user1"))
+ assert(!securityManager.checkModifyPermissions("user2"))
// change so they match again
securityManager.setModifyAclsGroups(Seq("group2", "group3"))
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
