You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "mengw15 (via GitHub)" <gi...@apache.org> on 2026/08/01 07:05:35 UTC
[PR] test(amber): add unit test coverage for HubResource [texera]
mengw15 opened a new pull request, #7230:
URL: https://github.com/apache/texera/pull/7230
### What changes were proposed in this PR?
Adds a `MockTexeraDB`-backed spec for `HubResource` (previously ~0% coverage),
following the sibling embedded-Postgres specs. Seeds an owner + a liker user and a
public workflow (with ownership + access), mocks `HttpServletRequest`, and covers
the non-LakeFS surface with 11 tests:
- **Actions (object helpers):** `recordUserAction` writes a `user_action` row;
`recordLikeAction` inserts a like the first time / removes it on unlike / returns
`false` when already liked; `recordCloneAction` writes a clone row and an action
row.
- **Endpoints:** `postLike` / `postUnlike` / `isLiked` toggle the like; `getCount`
counts public workflows; `postView` increments the view count (1 → 2) and records
a view action; `getCounts` reports the like count; `getTops` lists the liked
public entity under the `"like"` key; `userAccess` returns the granted user ids;
`fetchDashboardWorkflowsByWids` returns the seeded workflow.
All tests use `EntityType.Workflow` (the workflow tables), which keeps them off the
LakeFS-backed dataset-size path. No production code was changed.
### Any related issues, documentation, discussions?
Closes #7225.
### How was this PR tested?
`sbt "WorkflowExecutionService/testOnly *HubResourceSpec"` — 11 passed, run twice
in full plus the `getTops` / `getCounts` tests once in isolation to rule out
order-dependence, against the embedded Postgres. The failure path was verified by
breaking an assertion (1 failed, non-zero exit), and `Test/scalafmtCheck` +
`Test/scalafix --check` are clean.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8 [1M context])
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(amber): add unit test coverage for HubResource [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7230:
URL: https://github.com/apache/texera/pull/7230#issuecomment-5150321303
<!-- texera-reviewer-suggestion -->
### Automated Reviewer Suggestions
Based on the `git blame` history of the changed files, we recommend the following reviewers:
- No candidates found from `git blame` history.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(amber): add unit test coverage for HubResource [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #7230:
URL: https://github.com/apache/texera/pull/7230#issuecomment-5150343203
<!-- texera-benchmarks-comment -->
## ⚠️ Benchmark changes need a look
🟢 0 better · 🔴 12 worse · ⚪ 3 noise (<±5%) · 0 without baseline
> Compared against main `18ebb0d` benchmarked on **this same runner**, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.
[Dashboard](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/30689032127)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🔴 | bs=10 sw=10 sl=64 | 481 | 0.293 | 19,734/30,535/30,535 us | 🔴 +16.4% / 🔴 +88.5% |
| 🔴 | bs=100 sw=10 sl=64 | 1,006 | 0.614 | 91,680/162,608/162,608 us | 🔴 +6.8% / 🔴 +46.8% |
| 🔴 | bs=1000 sw=10 sl=64 | 1,192 | 0.727 | 841,209/905,165/905,165 us | 🔴 +8.3% / 🟢 +20.1% |
<details><summary>Baseline details</summary>
Latest main `18ebb0d` from same runner
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 481 tuples/sec | 539 tuples/sec | 765.25 tuples/sec | -10.8% | -37.1% |
| bs=10 sw=10 sl=64 | MB/s | 0.293 MB/s | 0.329 MB/s | 0.467 MB/s | -10.9% | -37.3% |
| bs=10 sw=10 sl=64 | p50 | 19,734 us | 17,408 us | 12,703 us | +13.4% | +55.3% |
| bs=10 sw=10 sl=64 | p95 | 30,535 us | 26,233 us | 16,199 us | +16.4% | +88.5% |
| bs=10 sw=10 sl=64 | p99 | 30,535 us | 26,233 us | 19,557 us | +16.4% | +56.1% |
| bs=100 sw=10 sl=64 | throughput | 1,006 tuples/sec | 1,019 tuples/sec | 965.22 tuples/sec | -1.3% | +4.2% |
| bs=100 sw=10 sl=64 | MB/s | 0.614 MB/s | 0.622 MB/s | 0.589 MB/s | -1.3% | +4.2% |
| bs=100 sw=10 sl=64 | p50 | 91,680 us | 92,027 us | 103,830 us | -0.4% | -11.7% |
| bs=100 sw=10 sl=64 | p95 | 162,608 us | 152,294 us | 110,778 us | +6.8% | +46.8% |
| bs=100 sw=10 sl=64 | p99 | 162,608 us | 152,294 us | 118,713 us | +6.8% | +37.0% |
| bs=1000 sw=10 sl=64 | throughput | 1,192 tuples/sec | 1,265 tuples/sec | 992.33 tuples/sec | -5.8% | +20.1% |
| bs=1000 sw=10 sl=64 | MB/s | 0.727 MB/s | 0.772 MB/s | 0.606 MB/s | -5.8% | +20.0% |
| bs=1000 sw=10 sl=64 | p50 | 841,209 us | 788,402 us | 1,016,959 us | +6.7% | -17.3% |
| bs=1000 sw=10 sl=64 | p95 | 905,165 us | 835,632 us | 1,062,501 us | +8.3% | -14.8% |
| bs=1000 sw=10 sl=64 | p99 | 905,165 us | 835,632 us | 1,082,611 us | +8.3% | -16.4% |
</details>
<details><summary>Raw CSV</summary>
```csv
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,415.93,200,128000,481,0.293,19733.53,30534.80,30534.80
1,100,10,64,20,1988.17,2000,1280000,1006,0.614,91680.39,162608.46,162608.46
2,1000,10,64,20,16782.68,20000,12800000,1192,0.727,841208.54,905165.19,905165.19
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(amber): add unit test coverage for HubResource [texera]
Posted by "mengw15 (via GitHub)" <gi...@apache.org>.
mengw15 commented on code in PR #7230:
URL: https://github.com/apache/texera/pull/7230#discussion_r3694962648
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala:
##########
@@ -0,0 +1,265 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.resource.dashboard.hub
+
+import org.apache.texera.auth.SessionUser
+import org.apache.texera.dao.MockTexeraDB
+import org.apache.texera.dao.jooq.generated.Tables._
+import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum
+import org.apache.texera.dao.jooq.generated.tables.daos.{
+ UserDao,
+ WorkflowDao,
+ WorkflowOfUserDao,
+ WorkflowUserAccessDao
+}
+import org.apache.texera.dao.jooq.generated.tables.pojos.{
+ User,
+ Workflow,
+ WorkflowOfUser,
+ WorkflowUserAccess
+}
+import org.apache.texera.web.resource.dashboard.hub.HubResource._
+import org.scalamock.scalatest.MockFactory
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+import java.sql.Timestamp
+import java.util.UUID
+import javax.servlet.http.HttpServletRequest
+import scala.jdk.CollectionConverters._
+
+class HubResourceSpec
+ extends AnyFlatSpec
+ with Matchers
+ with MockFactory
+ with BeforeAndAfterAll
+ with BeforeAndAfterEach
+ with MockTexeraDB {
+
+ private val ownerUid = 9000 + scala.util.Random.nextInt(500)
+ private val likerUid = 9500 + scala.util.Random.nextInt(400)
+ private val wid = 7000 + scala.util.Random.nextInt(1000)
Review Comment:
Done — switched to fixed ids (9001/9002/7001). Confirmed the isolation: `MockTexeraDB` gives each suite its own UUID-named database (`CREATE DATABASE texera_db_<uuid>`), so fixed ids can't collide across specs.
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala:
##########
@@ -0,0 +1,265 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.resource.dashboard.hub
+
+import org.apache.texera.auth.SessionUser
+import org.apache.texera.dao.MockTexeraDB
+import org.apache.texera.dao.jooq.generated.Tables._
+import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum
+import org.apache.texera.dao.jooq.generated.tables.daos.{
+ UserDao,
+ WorkflowDao,
+ WorkflowOfUserDao,
+ WorkflowUserAccessDao
+}
+import org.apache.texera.dao.jooq.generated.tables.pojos.{
+ User,
+ Workflow,
+ WorkflowOfUser,
+ WorkflowUserAccess
+}
+import org.apache.texera.web.resource.dashboard.hub.HubResource._
+import org.scalamock.scalatest.MockFactory
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+import java.sql.Timestamp
+import java.util.UUID
+import javax.servlet.http.HttpServletRequest
+import scala.jdk.CollectionConverters._
+
+class HubResourceSpec
+ extends AnyFlatSpec
+ with Matchers
+ with MockFactory
+ with BeforeAndAfterAll
+ with BeforeAndAfterEach
+ with MockTexeraDB {
+
+ private val ownerUid = 9000 + scala.util.Random.nextInt(500)
+ private val likerUid = 9500 + scala.util.Random.nextInt(400)
+ private val wid = 7000 + scala.util.Random.nextInt(1000)
+ private val Wf: EntityType = EntityType.Workflow
+
+ private var userDao: UserDao = _
+ private var workflowDao: WorkflowDao = _
+ private var workflowOfUserDao: WorkflowOfUserDao = _
+ private var workflowUserAccessDao: WorkflowUserAccessDao = _
+
+ // A request whose remote address is a valid IPv4 so recordUserAction stores it.
+ private def req: HttpServletRequest = {
+ val r = stub[HttpServletRequest]
+ (r.getRemoteAddr _).when().returns("127.0.0.1")
+ r
+ }
+
+ private def session(uid: Int): SessionUser = {
+ val u = new User
+ u.setUid(Integer.valueOf(uid))
+ new SessionUser(u)
+ }
+
+ private def makeUser(uid: Int, name: String): User = {
+ val u = new User
+ u.setUid(Integer.valueOf(uid))
+ u.setName(name)
+ u.setEmail(s"$name@test.com")
+ u.setPassword("password")
+ u
+ }
+
+ private def ids(values: Int*): java.util.List[Integer] = values.map(Integer.valueOf).asJava
+ private def types(values: EntityType*): java.util.List[EntityType] = values.toList.asJava
+ private def actions(values: ActionType*): java.util.List[ActionType] = values.toList.asJava
+
+ private def likeRows: Int =
+ getDSLContext
+ .selectCount()
+ .from(WORKFLOW_USER_LIKES)
+ .where(WORKFLOW_USER_LIKES.WID.eq(wid).and(WORKFLOW_USER_LIKES.UID.eq(likerUid)))
+ .fetchOne(0, classOf[Integer])
+ .intValue()
+
+ private def cloneRows: Int =
+ getDSLContext
+ .selectCount()
+ .from(WORKFLOW_USER_CLONES)
+ .where(WORKFLOW_USER_CLONES.WID.eq(wid).and(WORKFLOW_USER_CLONES.UID.eq(likerUid)))
+ .fetchOne(0, classOf[Integer])
+ .intValue()
+
+ private def userActionRows: Int =
+ getDSLContext
+ .selectCount()
+ .from(USER_ACTION)
+ .where(USER_ACTION.RESOURCE_ID.eq(wid))
+ .fetchOne(0, classOf[Integer])
+ .intValue()
+
+ private def isLiked(uid: Int): Boolean =
+ isLikedHelper(Integer.valueOf(uid), ids(wid), types(Wf)).asScala.head.isLiked
+
+ override protected def beforeAll(): Unit = {
+ initializeDBAndReplaceDSLContext()
+ userDao = new UserDao(getDSLContext.configuration())
+ workflowDao = new WorkflowDao(getDSLContext.configuration())
+ workflowOfUserDao = new WorkflowOfUserDao(getDSLContext.configuration())
+ workflowUserAccessDao = new WorkflowUserAccessDao(getDSLContext.configuration())
+
+ userDao.insert(makeUser(ownerUid, "hub_owner"))
+ userDao.insert(makeUser(likerUid, "hub_liker"))
+
+ val wf = new Workflow
+ wf.setWid(Integer.valueOf(wid))
+ wf.setName("hub_wf_" + UUID.randomUUID().toString.substring(0, 8))
+ wf.setContent("{}")
+ wf.setDescription("hub test")
+ wf.setIsPublic(true)
+ wf.setCreationTime(new Timestamp(System.currentTimeMillis()))
+ wf.setLastModifiedTime(new Timestamp(System.currentTimeMillis()))
+ workflowDao.insert(wf)
+
+ val ownership = new WorkflowOfUser
+ ownership.setUid(Integer.valueOf(ownerUid))
+ ownership.setWid(Integer.valueOf(wid))
+ workflowOfUserDao.insert(ownership)
+
+ val access = new WorkflowUserAccess
+ access.setUid(Integer.valueOf(ownerUid))
+ access.setWid(Integer.valueOf(wid))
+ access.setPrivilege(PrivilegeEnum.WRITE)
+ workflowUserAccessDao.insert(access)
+ }
+
+ override protected def afterAll(): Unit = closeConnectionPool()
+
+ // Reset the per-entity hub rows so every test starts from a clean slate.
+ override protected def beforeEach(): Unit = {
+ val ctx = getDSLContext
+ ctx.deleteFrom(WORKFLOW_USER_LIKES).where(WORKFLOW_USER_LIKES.WID.eq(wid)).execute()
+ ctx.deleteFrom(WORKFLOW_USER_CLONES).where(WORKFLOW_USER_CLONES.WID.eq(wid)).execute()
+ ctx.deleteFrom(WORKFLOW_VIEW_COUNT).where(WORKFLOW_VIEW_COUNT.WID.eq(wid)).execute()
+ ctx.deleteFrom(USER_ACTION).where(USER_ACTION.RESOURCE_ID.eq(wid)).execute()
+ }
+
+ "recordUserAction" should "insert a user_action row for the entity" in {
+ recordUserAction(req, Integer.valueOf(likerUid), Integer.valueOf(wid), Wf, ActionType.Like)
+ userActionRows shouldBe 1
+ }
+
+ "recordLikeAction" should "insert a like the first time and remove it on unlike" in {
+ recordLikeAction(
+ req,
+ Integer.valueOf(likerUid),
+ UserRequest(wid, Wf),
+ isLike = true
+ ) shouldBe true
+ likeRows shouldBe 1
+ isLiked(likerUid) shouldBe true
+
+ recordLikeAction(
+ req,
+ Integer.valueOf(likerUid),
+ UserRequest(wid, Wf),
+ isLike = false
+ ) shouldBe true
+ likeRows shouldBe 0
+ isLiked(likerUid) shouldBe false
+ }
+
+ it should "return false when liking something already liked" in {
+ recordLikeAction(
+ req,
+ Integer.valueOf(likerUid),
+ UserRequest(wid, Wf),
+ isLike = true
+ ) shouldBe true
+ recordLikeAction(
+ req,
+ Integer.valueOf(likerUid),
+ UserRequest(wid, Wf),
+ isLike = true
+ ) shouldBe false
+ likeRows shouldBe 1
+ }
+
+ "recordCloneAction" should "record a clone row and a user_action row" in {
+ recordCloneAction(req, Integer.valueOf(likerUid), Integer.valueOf(wid), Wf)
+ cloneRows shouldBe 1
+ userActionRows should be >= 1
Review Comment:
Done — `shouldBe 1` (recordCloneAction records exactly one user_action row, cleared per-wid in beforeEach). Also tightened `getCount` to `shouldBe 1` and made `getTops` assert the seeded wid — all verified in isolation too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(amber): add unit test coverage for HubResource [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #7230:
URL: https://github.com/apache/texera/pull/7230#issuecomment-5150367292
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/7230?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 81.29%. Comparing base ([`18ebb0d`](https://app.codecov.io/gh/apache/texera/commit/18ebb0d1e6dded6f8bc04b7c52be4fa31cb27916?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`d29d1b6`](https://app.codecov.io/gh/apache/texera/commit/d29d1b677f0d67fd5d7f9b8c1878fef40976b03e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on main.
:white_check_mark: All tests successful. No failed tests found.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #7230 +/- ##
============================================
+ Coverage 81.04% 81.29% +0.25%
- Complexity 3968 3974 +6
============================================
Files 1162 1162
Lines 46259 46259
Branches 5153 5153
============================================
+ Hits 37490 37606 +116
+ Misses 7096 6961 -135
- Partials 1673 1692 +19
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/7230/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/7230/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.00% <ø> (ø)` | | Carriedforward from [18ebb0d](https://app.codecov.io/gh/apache/texera/commit/18ebb0d1e6dded6f8bc04b7c52be4fa31cb27916?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/7230/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.49% <ø> (ø)` | | Carriedforward from [18ebb0d](https://app.codecov.io/gh/apache/texera/commit/18ebb0d1e6dded6f8bc04b7c52be4fa31cb27916?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/7230/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.34% <ø> (+0.68%)` | :arrow_up: | |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/7230/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `20.49% <ø> (ø)` | | Carriedforward from [18ebb0d](https://app.codecov.io/gh/apache/texera/commit/18ebb0d1e6dded6f8bc04b7c52be4fa31cb27916?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/7230/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.97% <ø> (ø)` | | Carriedforward from [18ebb0d](https://app.codecov.io/gh/apache/texera/commit/18ebb0d1e6dded6f8bc04b7c52be4fa31cb27916?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/7230/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `66.80% <ø> (ø)` | | Carriedforward from [18ebb0d](https://app.codecov.io/gh/apache/texera/commit/18ebb0d1e6dded6f8bc04b7c52be4fa31cb27916?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/7230/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `83.22% <ø> (ø)` | | Carriedforward from [18ebb0d](https://app.codecov.io/gh/apache/texera/commit/18ebb0d1e6dded6f8bc04b7c52be4fa31cb27916?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [notebook-migration-service](https://app.codecov.io/gh/apache/texera/pull/7230/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `78.89% <ø> (ø)` | | Carriedforward from [18ebb0d](https://app.codecov.io/gh/apache/texera/commit/18ebb0d1e6dded6f8bc04b7c52be4fa31cb27916?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/7230/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `97.36% <ø> (ø)` | | Carriedforward from [18ebb0d](https://app.codecov.io/gh/apache/texera/commit/18ebb0d1e6dded6f8bc04b7c52be4fa31cb27916?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/7230/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `26.31% <ø> (ø)` | | Carriedforward from [18ebb0d](https://app.codecov.io/gh/apache/texera/commit/18ebb0d1e6dded6f8bc04b7c52be4fa31cb27916?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/7230?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(amber): add unit test coverage for HubResource [texera]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7230:
URL: https://github.com/apache/texera/pull/7230#discussion_r3694917838
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala:
##########
@@ -0,0 +1,265 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.resource.dashboard.hub
+
+import org.apache.texera.auth.SessionUser
+import org.apache.texera.dao.MockTexeraDB
+import org.apache.texera.dao.jooq.generated.Tables._
+import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum
+import org.apache.texera.dao.jooq.generated.tables.daos.{
+ UserDao,
+ WorkflowDao,
+ WorkflowOfUserDao,
+ WorkflowUserAccessDao
+}
+import org.apache.texera.dao.jooq.generated.tables.pojos.{
+ User,
+ Workflow,
+ WorkflowOfUser,
+ WorkflowUserAccess
+}
+import org.apache.texera.web.resource.dashboard.hub.HubResource._
+import org.scalamock.scalatest.MockFactory
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+import java.sql.Timestamp
+import java.util.UUID
+import javax.servlet.http.HttpServletRequest
+import scala.jdk.CollectionConverters._
+
+class HubResourceSpec
+ extends AnyFlatSpec
+ with Matchers
+ with MockFactory
+ with BeforeAndAfterAll
+ with BeforeAndAfterEach
+ with MockTexeraDB {
+
+ private val ownerUid = 9000 + scala.util.Random.nextInt(500)
+ private val likerUid = 9500 + scala.util.Random.nextInt(400)
+ private val wid = 7000 + scala.util.Random.nextInt(1000)
Review Comment:
Using randomly generated IDs makes failures harder to reproduce and isn’t needed here because MockTexeraDB creates an isolated per-suite database. Prefer fixed IDs for deterministic, debuggable fixtures.
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala:
##########
@@ -0,0 +1,265 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.resource.dashboard.hub
+
+import org.apache.texera.auth.SessionUser
+import org.apache.texera.dao.MockTexeraDB
+import org.apache.texera.dao.jooq.generated.Tables._
+import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum
+import org.apache.texera.dao.jooq.generated.tables.daos.{
+ UserDao,
+ WorkflowDao,
+ WorkflowOfUserDao,
+ WorkflowUserAccessDao
+}
+import org.apache.texera.dao.jooq.generated.tables.pojos.{
+ User,
+ Workflow,
+ WorkflowOfUser,
+ WorkflowUserAccess
+}
+import org.apache.texera.web.resource.dashboard.hub.HubResource._
+import org.scalamock.scalatest.MockFactory
+import org.scalatest.BeforeAndAfterAll
+import org.scalatest.BeforeAndAfterEach
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+
+import java.sql.Timestamp
+import java.util.UUID
+import javax.servlet.http.HttpServletRequest
+import scala.jdk.CollectionConverters._
+
+class HubResourceSpec
+ extends AnyFlatSpec
+ with Matchers
+ with MockFactory
+ with BeforeAndAfterAll
+ with BeforeAndAfterEach
+ with MockTexeraDB {
+
+ private val ownerUid = 9000 + scala.util.Random.nextInt(500)
+ private val likerUid = 9500 + scala.util.Random.nextInt(400)
+ private val wid = 7000 + scala.util.Random.nextInt(1000)
+ private val Wf: EntityType = EntityType.Workflow
+
+ private var userDao: UserDao = _
+ private var workflowDao: WorkflowDao = _
+ private var workflowOfUserDao: WorkflowOfUserDao = _
+ private var workflowUserAccessDao: WorkflowUserAccessDao = _
+
+ // A request whose remote address is a valid IPv4 so recordUserAction stores it.
+ private def req: HttpServletRequest = {
+ val r = stub[HttpServletRequest]
+ (r.getRemoteAddr _).when().returns("127.0.0.1")
+ r
+ }
+
+ private def session(uid: Int): SessionUser = {
+ val u = new User
+ u.setUid(Integer.valueOf(uid))
+ new SessionUser(u)
+ }
+
+ private def makeUser(uid: Int, name: String): User = {
+ val u = new User
+ u.setUid(Integer.valueOf(uid))
+ u.setName(name)
+ u.setEmail(s"$name@test.com")
+ u.setPassword("password")
+ u
+ }
+
+ private def ids(values: Int*): java.util.List[Integer] = values.map(Integer.valueOf).asJava
+ private def types(values: EntityType*): java.util.List[EntityType] = values.toList.asJava
+ private def actions(values: ActionType*): java.util.List[ActionType] = values.toList.asJava
+
+ private def likeRows: Int =
+ getDSLContext
+ .selectCount()
+ .from(WORKFLOW_USER_LIKES)
+ .where(WORKFLOW_USER_LIKES.WID.eq(wid).and(WORKFLOW_USER_LIKES.UID.eq(likerUid)))
+ .fetchOne(0, classOf[Integer])
+ .intValue()
+
+ private def cloneRows: Int =
+ getDSLContext
+ .selectCount()
+ .from(WORKFLOW_USER_CLONES)
+ .where(WORKFLOW_USER_CLONES.WID.eq(wid).and(WORKFLOW_USER_CLONES.UID.eq(likerUid)))
+ .fetchOne(0, classOf[Integer])
+ .intValue()
+
+ private def userActionRows: Int =
+ getDSLContext
+ .selectCount()
+ .from(USER_ACTION)
+ .where(USER_ACTION.RESOURCE_ID.eq(wid))
+ .fetchOne(0, classOf[Integer])
+ .intValue()
+
+ private def isLiked(uid: Int): Boolean =
+ isLikedHelper(Integer.valueOf(uid), ids(wid), types(Wf)).asScala.head.isLiked
+
+ override protected def beforeAll(): Unit = {
+ initializeDBAndReplaceDSLContext()
+ userDao = new UserDao(getDSLContext.configuration())
+ workflowDao = new WorkflowDao(getDSLContext.configuration())
+ workflowOfUserDao = new WorkflowOfUserDao(getDSLContext.configuration())
+ workflowUserAccessDao = new WorkflowUserAccessDao(getDSLContext.configuration())
+
+ userDao.insert(makeUser(ownerUid, "hub_owner"))
+ userDao.insert(makeUser(likerUid, "hub_liker"))
+
+ val wf = new Workflow
+ wf.setWid(Integer.valueOf(wid))
+ wf.setName("hub_wf_" + UUID.randomUUID().toString.substring(0, 8))
+ wf.setContent("{}")
+ wf.setDescription("hub test")
+ wf.setIsPublic(true)
+ wf.setCreationTime(new Timestamp(System.currentTimeMillis()))
+ wf.setLastModifiedTime(new Timestamp(System.currentTimeMillis()))
+ workflowDao.insert(wf)
+
+ val ownership = new WorkflowOfUser
+ ownership.setUid(Integer.valueOf(ownerUid))
+ ownership.setWid(Integer.valueOf(wid))
+ workflowOfUserDao.insert(ownership)
+
+ val access = new WorkflowUserAccess
+ access.setUid(Integer.valueOf(ownerUid))
+ access.setWid(Integer.valueOf(wid))
+ access.setPrivilege(PrivilegeEnum.WRITE)
+ workflowUserAccessDao.insert(access)
+ }
+
+ override protected def afterAll(): Unit = closeConnectionPool()
+
+ // Reset the per-entity hub rows so every test starts from a clean slate.
+ override protected def beforeEach(): Unit = {
+ val ctx = getDSLContext
+ ctx.deleteFrom(WORKFLOW_USER_LIKES).where(WORKFLOW_USER_LIKES.WID.eq(wid)).execute()
+ ctx.deleteFrom(WORKFLOW_USER_CLONES).where(WORKFLOW_USER_CLONES.WID.eq(wid)).execute()
+ ctx.deleteFrom(WORKFLOW_VIEW_COUNT).where(WORKFLOW_VIEW_COUNT.WID.eq(wid)).execute()
+ ctx.deleteFrom(USER_ACTION).where(USER_ACTION.RESOURCE_ID.eq(wid)).execute()
+ }
+
+ "recordUserAction" should "insert a user_action row for the entity" in {
+ recordUserAction(req, Integer.valueOf(likerUid), Integer.valueOf(wid), Wf, ActionType.Like)
+ userActionRows shouldBe 1
+ }
+
+ "recordLikeAction" should "insert a like the first time and remove it on unlike" in {
+ recordLikeAction(
+ req,
+ Integer.valueOf(likerUid),
+ UserRequest(wid, Wf),
+ isLike = true
+ ) shouldBe true
+ likeRows shouldBe 1
+ isLiked(likerUid) shouldBe true
+
+ recordLikeAction(
+ req,
+ Integer.valueOf(likerUid),
+ UserRequest(wid, Wf),
+ isLike = false
+ ) shouldBe true
+ likeRows shouldBe 0
+ isLiked(likerUid) shouldBe false
+ }
+
+ it should "return false when liking something already liked" in {
+ recordLikeAction(
+ req,
+ Integer.valueOf(likerUid),
+ UserRequest(wid, Wf),
+ isLike = true
+ ) shouldBe true
+ recordLikeAction(
+ req,
+ Integer.valueOf(likerUid),
+ UserRequest(wid, Wf),
+ isLike = true
+ ) shouldBe false
+ likeRows shouldBe 1
+ }
+
+ "recordCloneAction" should "record a clone row and a user_action row" in {
+ recordCloneAction(req, Integer.valueOf(likerUid), Integer.valueOf(wid), Wf)
+ cloneRows shouldBe 1
+ userActionRows should be >= 1
Review Comment:
This assertion is weaker than necessary: beforeEach clears USER_ACTION rows for this wid, and recordCloneAction always records exactly one user_action row. Tightening it to an exact count makes the test better at catching regressions (e.g., duplicate action writes).
This issue also appears in the following locations of the same file:
- line 220
- line 249
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] test(amber): add unit test coverage for HubResource [texera]
Posted by "aglinxinyuan (via GitHub)" <gi...@apache.org>.
aglinxinyuan merged PR #7230:
URL: https://github.com/apache/texera/pull/7230
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org