Skip to content

Commit

Permalink
Fix broken tests, and add them to CI
Browse files Browse the repository at this point in the history
Remove Dagger tests, as examples handles them

Signed-off-by: restingbull@cabaretmechanique.com <restingbull@cabaretmechanique.com>
  • Loading branch information
restingbull committed Aug 2, 2021
1 parent cb98782 commit fb41786
Show file tree
Hide file tree
Showing 17 changed files with 109 additions and 106 deletions.
9 changes: 9 additions & 0 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,15 @@ tasks:
- test
build_targets:
- //...
examples-dagger:
name: "Example - Dagger"
platform: ubuntu1804
working_directory: examples/dagger
include_json_profile:
- build
- test
build_targets:
- //...
examples-nodejs:
name: Example - Node
platform: ubuntu1804
Expand Down
2 changes: 1 addition & 1 deletion .bazelignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
# Exclude examples from //...:all
examples/*
examples
7 changes: 1 addition & 6 deletions .bazelproject
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,11 @@
directories:
# Add the directories you want added as source here
# By default, we've added your entire workspace ('.')
-examples/node/bazel-node
-examples/node/bazel-bin
-examples/node/bazel-genfiles
-examples/node/bazel-out
-examples/node/bazel-testlogs
-examples/*
.

targets:
//:all_local_tests
//examples/dagger/...
# These targets are built for the ide only. Primary purpose is to ensure the builder can build the targets, but it's
# also a good way of testing the intellij plugin.
//src/main/kotlin/io/bazel/kotlin/builder/tasks:tasks_for_ide
Expand Down
2 changes: 2 additions & 0 deletions scripts/release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ if test ! -d examples; then
fail "unable to find example directory: $PWD"
fi

bazel test //...:all || fail "tests failed"

# build release
bazel build //:rules_kotlin_release || fail "release archive failed"

Expand Down
20 changes: 16 additions & 4 deletions src/main/kotlin/io/bazel/worker/ContextLog.kt
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,31 @@ import java.nio.charset.StandardCharsets.UTF_8
import java.util.logging.Level

/** Log encapsulates standard out and error of execution. */
data class ContextLog(val out: CharSequence, val profiles: List<String> = emptyList()) :
data class ContextLog(
val out: CharSequence,
val profiles: List<String> = emptyList()
) :
CharSequence by out {
constructor(bytes: ByteArray, profiles: List<String>) : this(String(bytes, UTF_8), profiles)
constructor(
bytes: ByteArray,
profiles: List<String>
) : this(String(bytes, UTF_8), profiles)

enum class Granularity(val level: Level) {
INFO(Level.INFO), ERROR(Level.SEVERE), DEBUG(Level.FINEST)
INFO(Level.INFO),
ERROR(Level.SEVERE),
DEBUG(Level.FINEST)
}

/** Logging runtime messages lazily */
interface Logging {
fun debug(msg: () -> String)
fun info(msg: () -> String)
fun error(t: Throwable, msg: () -> String)
fun error(
t: Throwable,
msg: () -> String
)

fun error(msg: () -> String)
}

Expand Down
9 changes: 5 additions & 4 deletions src/main/kotlin/io/bazel/worker/CpuTimeBasedGcScheduler.kt
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.bazel.worker

import com.sun.management.OperatingSystemMXBean
import src.main.kotlin.io.bazel.worker.GcScheduler
import java.lang.management.ManagementFactory
import java.time.Duration
import java.util.concurrent.atomic.AtomicReference
Expand All @@ -12,21 +13,21 @@ class CpuTimeBasedGcScheduler(
* disable.
*/
private val cpuUsageBeforeGc: Duration
) {
) : GcScheduler {

/** The total process CPU time at the last GC run (or from the start of the worker). */
private val cpuTime: Duration
get() = if (cpuUsageBeforeGc.isZero) Duration.ZERO else Duration.ofNanos(bean.processCpuTime)
private val cpuTimeAtLastGc: AtomicReference<Duration> = AtomicReference(cpuTime)

/** Call occasionally to perform a GC if enough CPU time has been used. */
fun maybePerformGc() {
override fun maybePerformGc() {
if (!cpuUsageBeforeGc.isZero) {
val currentCpuTime = cpuTime
val lastCpuTime = cpuTimeAtLastGc.get()
// Do GC when enough CPU time has been used, but only if nobody else beat us to it.
if (currentCpuTime.minus(lastCpuTime).compareTo(cpuUsageBeforeGc) > 0
&& cpuTimeAtLastGc.compareAndSet(lastCpuTime, currentCpuTime)
if (currentCpuTime.minus(lastCpuTime).compareTo(cpuUsageBeforeGc) > 0 &&
cpuTimeAtLastGc.compareAndSet(lastCpuTime, currentCpuTime)
) {
System.gc()
// Avoid counting GC CPU time against CPU time before next GC.
Expand Down
6 changes: 6 additions & 0 deletions src/main/kotlin/io/bazel/worker/GcScheduler.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package src.main.kotlin.io.bazel.worker

/** GcScheduler for invoking garbage collection in a persistent worker. */
fun interface GcScheduler {
fun maybePerformGc()
}
1 change: 1 addition & 0 deletions src/main/kotlin/io/bazel/worker/IO.kt
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class IO(
* the same console output multiple times
**/
fun readCapturedAsUtf8String(): String {
captured.flush()
val out = captured.toByteArray().toString(StandardCharsets.UTF_8)
captured.reset()
return out
Expand Down
14 changes: 9 additions & 5 deletions src/main/kotlin/io/bazel/worker/InvocationWorker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,14 @@ package io.bazel.worker
/** InvocationWorker executes a single unit of work. */
class InvocationWorker(private val arguments: Iterable<String>) : Worker {
override fun start(execute: Work): Int =
WorkerContext.run {
doTask("invocation") { ctx -> execute(ctx, arguments) }.run {
println(log.out.toString())
status.exit
runCatching {
WorkerContext.run {
doTask("invocation") { ctx -> execute(ctx, arguments) }.run {
println(log.out.toString())
status.exit
}
}
}
}.recover {
1
}.getOrThrow()
}
87 changes: 43 additions & 44 deletions src/main/kotlin/io/bazel/worker/PersistentWorker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -19,30 +19,18 @@ package io.bazel.worker

import com.google.devtools.build.lib.worker.WorkerProtocol
import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExecutorCoroutineDispatcher
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.asCoroutineDispatcher
import kotlinx.coroutines.async
import kotlinx.coroutines.flow.asFlow
import kotlinx.coroutines.flow.buffer
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.flowOn
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.Job
import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.channels.Channel.Factory.UNLIMITED
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.consumeAsFlow
import kotlinx.coroutines.flow.onCompletion
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withContext
import src.main.kotlin.io.bazel.worker.GcScheduler
import java.io.PrintStream
import java.nio.charset.StandardCharsets.UTF_8
import java.time.Duration
import java.util.concurrent.Executors
import kotlin.coroutines.CoroutineContext

/**
Expand All @@ -57,27 +45,34 @@ import kotlin.coroutines.CoroutineContext
class PersistentWorker(
private val coroutineContext: CoroutineContext,
private val captureIO: () -> IO,
private val cpuTimeBasedGcScheduler: CpuTimeBasedGcScheduler,
private val cpuTimeBasedGcScheduler: GcScheduler
) : Worker {

constructor() : this(Dispatchers.IO, IO.Companion::capture, CpuTimeBasedGcScheduler(Duration.ofSeconds(10)))
constructor(
coroutineContext: CoroutineContext,
captureIO: () -> IO
) : this(coroutineContext, captureIO, GcScheduler {})

constructor() : this(
Dispatchers.IO, IO.Companion::capture, CpuTimeBasedGcScheduler(Duration.ofSeconds(10))
)

@ExperimentalCoroutinesApi
override fun start(execute: Work) = WorkerContext.run {
//Use channel to serialize writing output
// Use channel to serialize writing output
val writeChannel = Channel<WorkerProtocol.WorkResponse>(UNLIMITED)
captureIO().use { io ->
runBlocking {
//Parent coroutine to track all of children and close channel on completion
// Parent coroutine to track all of children and close channel on completion
launch(Dispatchers.Default) {
generateSequence { WorkRequest.parseDelimitedFrom(io.input) }
.forEach { request ->
launch {
compileWork(request, io, writeChannel, execute)
//Be a friendly worker by performing a GC between compilation requests
cpuTimeBasedGcScheduler.maybePerformGc()
}
generateSequence { WorkRequest.parseDelimitedFrom(io.input) }
.forEach { request ->
launch {
compileWork(request, io, writeChannel, execute)
// Be a friendly worker by performing a GC between compilation requests
cpuTimeBasedGcScheduler.maybePerformGc()
}
}
}.invokeOnCompletion { writeChannel.close() }

writeChannel.consumeAsFlow()
Expand All @@ -96,30 +91,34 @@ class PersistentWorker(
chan: Channel<WorkerProtocol.WorkResponse>,
execute: Work
) = withContext(Dispatchers.Default) {
val result = doTask("request ${request.requestId}") { ctx ->
request.argumentsList.run {
execute(ctx, toList())
}
}
info { "task result ${result.status}" }
val response = WorkerProtocol.WorkResponse.newBuilder().apply {
output = listOf(
result.log.out.toString(),
io.readCapturedAsUtf8String()
).filter { it.isNotBlank() }.joinToString("\n")
exitCode = result.status.exit
requestId = request.requestId
}.build()
info {
response.toString()
val result = doTask("request ${request.requestId}") { ctx ->
request.argumentsList.run {
execute(ctx, toList())
}
chan.send(response)
}
info { "task result ${result.status}" }
val response = WorkerProtocol.WorkResponse.newBuilder().apply {
val cap = io.readCapturedAsUtf8String()
output = listOf(
result.log.out.toString(),
cap
).joinToString("\n")
exitCode = result.status.exit
requestId = request.requestId
}.build()

info {
response.toString()
}
chan.send(response)
}

private suspend fun writeOutput(response: WorkerProtocol.WorkResponse, output: PrintStream) =
private suspend fun writeOutput(
response: WorkerProtocol.WorkResponse,
output: PrintStream
) =
withContext(Dispatchers.IO) {
response.writeDelimitedTo(output)
output.flush()
}

}
5 changes: 4 additions & 1 deletion src/main/kotlin/io/bazel/worker/TaskResult.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,7 @@

package io.bazel.worker

data class TaskResult(val status: Status, val log: ContextLog)
data class TaskResult(
val status: Status,
val log: ContextLog
)
5 changes: 4 additions & 1 deletion src/main/kotlin/io/bazel/worker/Work.kt
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,8 @@ import io.bazel.worker.WorkerContext.TaskContext

/** Task for Worker execution. */
fun interface Work {
operator fun invoke(ctx: TaskContext, args: Iterable<String>): Status
operator fun invoke(
ctx: TaskContext,
args: Iterable<String>
): Status
}
5 changes: 4 additions & 1 deletion src/main/kotlin/io/bazel/worker/Worker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ package io.bazel.worker
/** Worker executes a unit of Work */
interface Worker {
companion object {
fun from(args: Iterable<String>, then: Worker.(Iterable<String>) -> Int): Int {
fun from(
args: Iterable<String>,
then: Worker.(Iterable<String>) -> Int
): Int {
val worker = when {
"--persistent_worker" in args -> PersistentWorker()
else -> InvocationWorker(args)
Expand Down
5 changes: 4 additions & 1 deletion src/main/kotlin/io/bazel/worker/WorkerContext.kt
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,10 @@ class WorkerContext private constructor(
logger.logp(Level.INFO, sourceName, name, msg)
}

override fun error(t: Throwable, msg: () -> String) {
override fun error(
t: Throwable,
msg: () -> String
) {
logger.logp(Level.SEVERE, sourceName, name, t, msg)
}

Expand Down
6 changes: 0 additions & 6 deletions src/test/kotlin/io/bazel/kotlin/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,6 @@ kt_rules_e2e_test(
data = ["//src/test/data/jvm/kapt"],
)

kt_rules_e2e_test(
name = "KotlinJvmDaggerExampleTest",
srcs = ["KotlinJvmDaggerExampleTest.kt"],
data = ["//examples/dagger:coffee_app"],
)

kt_rules_e2e_test(
name = "KotlinJvmAssociatesBasicVisibilityTest",
Expand All @@ -74,7 +69,6 @@ test_suite(
"KotlinJvm13Test",
"KotlinJvmAssociatesBasicVisibilityTest",
"KotlinJvmBasicAssertionTest",
"KotlinJvmDaggerExampleTest",
"KotlinJvmKaptAssertionTest",
],
)
Expand Down
31 changes: 0 additions & 31 deletions src/test/kotlin/io/bazel/kotlin/KotlinJvmDaggerExampleTest.kt

This file was deleted.

1 change: 0 additions & 1 deletion src/test/kotlin/io/bazel/worker/PersistentWorkerTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ class PersistentWorkerTest {
closeStdIn()
return@inProcess generateSequence { readStdOut() }
}.associateBy { wr -> wr.requestId }
assertThat(String(captured.toByteArray(), UTF_8)).contains("Squeek!")

assertThat(actualResponses.keys).isEqualTo(expectedResponses.keys)

Expand Down

0 comments on commit fb41786

Please sign in to comment.