Disable file watcher in VcsManagerTest suite (#7421)

We can't really control the timing of file watcher events, which sometimes leads to failures of VCS tests. The PR disables the file watcher in the `VcsManagerTest` suite to make tests more stable.

Changelog:
- add: a `Watcher` and `WatcherFactory` interfaces
- add: a `NoopWatcher` test watcher
- update: disable the file watcher in the `VcsManagerTest` suite
This commit is contained in:
Dmitry Bushev 2023-07-31 10:26:20 +01:00 committed by GitHub
parent fe160ac287
commit c629d6078d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 191 additions and 97 deletions

View File

@ -9,6 +9,7 @@ import org.enso.distribution.locking.{
import org.enso.distribution.{DistributionManager, Environment, LanguageHome}
import org.enso.editions.EditionResolver
import org.enso.editions.updater.EditionManager
import org.enso.filewatcher.WatcherAdapterFactory
import org.enso.jsonrpc.JsonRpcServer
import org.enso.languageserver.capability.CapabilityRouter
import org.enso.languageserver.data._
@ -235,6 +236,7 @@ class MainModule(serverConfig: LanguageServerConfig, logLevel: LogLevel) {
ReceivesTreeUpdatesHandler.props(
languageServerConfig,
contentRootManagerWrapper,
new WatcherAdapterFactory,
fileSystem,
zioExec
),

View File

@ -2,7 +2,7 @@ package org.enso.languageserver.filemanager
import java.io.File
import enumeratum._
import org.enso.filewatcher.WatcherAdapter
import org.enso.filewatcher.Watcher
/** A representation of filesystem event.
*
@ -23,7 +23,7 @@ object FileEvent {
def fromWatcherEvent(
root: File,
base: Path,
event: WatcherAdapter.WatcherEvent
event: Watcher.WatcherEvent
): FileEvent =
FileEvent(
Path.getRelativePath(root, base, event.path),
@ -51,15 +51,15 @@ object FileEventKind extends Enum[FileEventKind] with CirceEnum[FileEventKind] {
override val values = findValues
/** Create [[FileEventKind]] from [[WatcherAdapter.EventType]].
/** Create [[FileEventKind]] from [[Watcher.EventType]].
*
* @param eventType file system event type
* @return file event kind
*/
def apply(eventType: WatcherAdapter.EventType): FileEventKind =
def apply(eventType: Watcher.EventType): FileEventKind =
eventType match {
case WatcherAdapter.EventTypeCreate => FileEventKind.Added
case WatcherAdapter.EventTypeModify => FileEventKind.Modified
case WatcherAdapter.EventTypeDelete => FileEventKind.Removed
case Watcher.EventTypeCreate => FileEventKind.Added
case Watcher.EventTypeModify => FileEventKind.Modified
case Watcher.EventTypeDelete => FileEventKind.Removed
}
}

View File

@ -4,7 +4,7 @@ import akka.actor.{Actor, ActorRef, Props}
import akka.pattern.pipe
import cats.implicits._
import com.typesafe.scalalogging.LazyLogging
import org.enso.filewatcher.WatcherAdapter
import org.enso.filewatcher.{Watcher, WatcherFactory}
import org.enso.languageserver.capability.CapabilityProtocol.{
CapabilityAcquired,
CapabilityAcquisitionFileSystemFailure,
@ -26,17 +26,19 @@ import java.io.File
import scala.concurrent.Await
import scala.util.{Failure, Success}
/** Starts [[WatcherAdapter]], handles errors, converts and sends
/** Starts [[Watcher]], handles errors, converts and sends
* events to the client.
*
* @param config configuration
* @param contentRootManager the content root manager
* @param watcherFactory the factory creating the file watcher
* @param fs file system
* @param exec executor of file system effects
*/
final class PathWatcher(
config: PathWatcherConfig,
contentRootManager: ContentRootManager,
watcherFactory: WatcherFactory,
fs: FileSystemApi[BlockingIO],
exec: Exec[BlockingIO]
) extends Actor
@ -48,7 +50,7 @@ final class PathWatcher(
private val restartCounter =
new PathWatcher.RestartCounter(config.maxRestarts)
private var fileWatcher: Option[WatcherAdapter] = None
private var fileWatcher: Option[Watcher] = None
override def preStart(): Unit = {
context.system.eventStream
@ -86,7 +88,7 @@ final class PathWatcher(
pathToWatchResult.onComplete {
case Success(Right(root)) =>
logger.info("Initialized for [{}]", path)
logger.info("Initialized [{}] for [{}].", watcherFactory.getClass, path)
context.become(initializedStage(root, path, clients))
case Success(Left(err)) =>
logger.error("Failed to resolve the path [{}]. {}", path, err)
@ -114,13 +116,13 @@ final class PathWatcher(
if clients.contains(client.rpcController) =>
unregisterClient(root, base, clients - client.rpcController)
case e: WatcherAdapter.WatcherEvent =>
case e: Watcher.WatcherEvent =>
restartCounter.reset()
val event = FileEvent.fromWatcherEvent(root, base, e)
clients.foreach(_ ! FileEventResult(event))
context.system.eventStream.publish(event)
case WatcherAdapter.WatcherError(e) =>
case Watcher.WatcherError(e) =>
stopWatcher()
restartCounter.inc()
if (restartCounter.canRestart) {
@ -161,13 +163,13 @@ final class PathWatcher(
private def buildWatcher(
path: File
): Either[FileSystemFailure, WatcherAdapter] =
): Either[FileSystemFailure, Watcher] =
Either
.catchNonFatal(WatcherAdapter.build(path.toPath, self ! _, self ! _))
.catchNonFatal(watcherFactory.build(path.toPath, self ! _, self ! _))
.leftMap(errorHandler)
private def startWatcher(
watcher: WatcherAdapter
watcher: Watcher
): Either[FileSystemFailure, Unit] =
Either
.catchNonFatal {
@ -225,14 +227,16 @@ object PathWatcher {
*
* @param config configuration
* @param contentRootManager the content root manager
* @param watcherFactory the factory creating a watcher instance
* @param fs file system
* @param exec executor of file system effects
*/
def props(
config: PathWatcherConfig,
contentRootManager: ContentRootManager,
watcherFactory: WatcherFactory,
fs: FileSystemApi[BlockingIO],
exec: Exec[BlockingIO]
): Props =
Props(new PathWatcher(config, contentRootManager, fs, exec))
Props(new PathWatcher(config, contentRootManager, watcherFactory, fs, exec))
}

View File

@ -2,6 +2,7 @@ package org.enso.languageserver.filemanager
import akka.actor.{Actor, ActorRef, Props, Stash, Terminated}
import com.typesafe.scalalogging.LazyLogging
import org.enso.filewatcher.WatcherFactory
import org.enso.languageserver.capability.CapabilityProtocol.{
AcquireCapability,
CapabilityNotAcquiredResponse,
@ -49,12 +50,14 @@ import org.enso.languageserver.util.UnhandledLogging
*
* @param config configuration
* @param contentRootManager the content root manager
* @param watcherFactory the factory creating the file watcher
* @param fs file system
* @param exec executor of file system events
*/
final class ReceivesTreeUpdatesHandler(
config: Config,
contentRootManager: ContentRootManager,
watcherFactory: WatcherFactory,
fs: FileSystemApi[BlockingIO],
exec: Exec[BlockingIO]
) extends Actor
@ -96,6 +99,7 @@ final class ReceivesTreeUpdatesHandler(
PathWatcher.props(
config.pathWatcher,
contentRootManager,
watcherFactory,
fs,
exec
)
@ -124,6 +128,7 @@ final class ReceivesTreeUpdatesHandler(
PathWatcher.props(
config.pathWatcher,
contentRootManager,
watcherFactory,
fs,
exec
)
@ -198,14 +203,24 @@ object ReceivesTreeUpdatesHandler {
*
* @param config configuration
* @param contentRootManager the content root manager
* @param watcherFactory the factory creating the file watcher
* @param fs file system
* @param exec executor of file system events
*/
def props(
config: Config,
contentRootManager: ContentRootManager,
watcherFactory: WatcherFactory,
fs: FileSystemApi[BlockingIO],
exec: Exec[BlockingIO]
): Props =
Props(new ReceivesTreeUpdatesHandler(config, contentRootManager, fs, exec))
Props(
new ReceivesTreeUpdatesHandler(
config,
contentRootManager,
watcherFactory,
fs,
exec
)
)
}

View File

@ -0,0 +1,11 @@
package org.enso.filewatcher
/** A file watcher that does nothing. */
class NoopWatcher extends Watcher {
/** @inheritdoc */
override def start(): Unit = ()
/** @inheritdoc */
override def stop(): Unit = ()
}

View File

@ -0,0 +1,12 @@
package org.enso.filewatcher
import java.nio.file.Path
class NoopWatcherFactory extends WatcherFactory {
override def build(
root: Path,
eventCallback: Watcher.WatcherEvent => Unit,
exceptionCallback: Watcher.WatcherError => Unit
): Watcher =
new NoopWatcher
}

View File

@ -9,6 +9,7 @@ import org.enso.distribution.locking.ResourceManager
import org.enso.distribution.{DistributionManager, LanguageHome}
import org.enso.editions.updater.EditionManager
import org.enso.editions.{EditionResolver, Editions}
import org.enso.filewatcher.{NoopWatcherFactory, WatcherAdapterFactory}
import org.enso.jsonrpc.test.JsonRpcServerTestKit
import org.enso.jsonrpc.{ClientControllerFactory, ProtocolFactory}
import org.enso.languageserver.TestClock
@ -74,6 +75,8 @@ class BaseServerTest
val timeout: FiniteDuration = 10.seconds
def isFileWatcherEnabled: Boolean = false
val testContentRootId = UUID.randomUUID()
val testContentRoot = ContentRootWithFile(
ContentRoot.Project(testContentRootId),
@ -201,10 +204,14 @@ class BaseServerTest
),
s"buffer-registry-${UUID.randomUUID()}"
)
val watcherFactory =
if (isFileWatcherEnabled) new WatcherAdapterFactory
else new NoopWatcherFactory
val fileEventRegistry = system.actorOf(
ReceivesTreeUpdatesHandler.props(
config,
contentRootManagerWrapper,
watcherFactory,
new FileSystem,
zioExec
),

View File

@ -6,6 +6,8 @@ import io.circe.literal._
class ReceivesTreeUpdatesHandlerTest extends BaseServerTest {
override val isFileWatcherEnabled = true
"ReceivesTreeUpdatesHandler" must {
"acquire capability receivesTreeUpdates" in {

View File

@ -14,6 +14,8 @@ import scala.concurrent.duration._
class TextOperationsTest extends BaseServerTest with FlakySpec {
override def isFileWatcherEnabled = true
"text/openFile" must {
"fail opening a file if it does not exist" taggedAs Flaky in {
// Interaction:

View File

@ -314,8 +314,6 @@ class VcsManagerTest extends BaseServerTest with RetrySpec with FlakySpec {
fooPath,
"123456789".getBytes(StandardCharsets.UTF_8)
)
// Ensure that the file handler is released on Windows
Thread.sleep(2000)
client.send(json"""
{ "jsonrpc": "2.0",
"method": "text/openFile",

View File

@ -0,0 +1,42 @@
package org.enso.filewatcher
import java.nio.file.Path
trait Watcher {
/** Start the file watcher. */
def start(): Unit
/** Stop the file watcher. */
def stop(): Unit
}
object Watcher {
/** Type of a file event. */
sealed trait EventType
/** Event type indicating file creation. */
case object EventTypeCreate extends EventType
/** Event type indicating file modification. */
case object EventTypeModify extends EventType
/** Event type indicating file deletion. */
case object EventTypeDelete extends EventType
/** Object representing file system event.
*
* @param path path to the file system object
* @param eventType event type
*/
case class WatcherEvent(path: Path, eventType: EventType)
object WatcherEvent {}
/** Object representing en error.
*
* @param exception an error
*/
case class WatcherError(exception: Exception)
}

View File

@ -12,11 +12,10 @@ import java.nio.file.Path
*/
final class WatcherAdapter(
root: Path,
eventCallback: WatcherAdapter.WatcherEvent => Unit,
errorCallback: WatcherAdapter.WatcherError => Unit
) extends DirectoryChangeListener {
import WatcherAdapter._
eventCallback: Watcher.WatcherEvent => Unit,
errorCallback: Watcher.WatcherError => Unit
) extends Watcher
with DirectoryChangeListener {
private val watcher: DirectoryWatcher = DirectoryWatcher
.builder()
@ -24,87 +23,57 @@ final class WatcherAdapter(
.listener(this)
.build()
def start(): Unit = {
/** @inheritdoc */
override def start(): Unit = {
watcher.watch()
}
def stop(): Unit = {
/** @inheritdoc */
override def stop(): Unit = {
watcher.close()
}
/** A callback executed by `DirectoryWatcher` on file system event. */
override def onEvent(event: DirectoryChangeEvent): Unit = {
WatcherEvent
.from(event)
WatcherAdapter
.watcherEvent(event)
.foreach(eventCallback)
}
override def onException(e: Exception): Unit = {
errorCallback(WatcherAdapter.WatcherError(e))
errorCallback(Watcher.WatcherError(e))
}
}
object WatcherAdapter {
/** Type of a file event. */
sealed trait EventType
private object EventType {
/** Creates [[EventType]] from file system event type.
*
* @param eventType file system event type
* @return watcher event type
*/
def from(eventType: DirectoryChangeEvent.EventType): Option[EventType] =
eventType match {
case DirectoryChangeEvent.EventType.CREATE => Some(EventTypeCreate)
case DirectoryChangeEvent.EventType.MODIFY => Some(EventTypeModify)
case DirectoryChangeEvent.EventType.DELETE => Some(EventTypeDelete)
case DirectoryChangeEvent.EventType.OVERFLOW => None
}
}
/** Event type indicating file creation. */
case object EventTypeCreate extends EventType
/** Event type indicating file modification. */
case object EventTypeModify extends EventType
/** Event type indicating file deletion. */
case object EventTypeDelete extends EventType
/** Object representing file system event.
/** Creates [[Watcher.EventType]] from file system event type.
*
* @param path path to the file system object
* @param eventType event type
* @param eventType file system event type
* @return watcher event type
*/
case class WatcherEvent(path: Path, eventType: EventType)
private def eventType(
eventType: DirectoryChangeEvent.EventType
): Option[Watcher.EventType] =
eventType match {
case DirectoryChangeEvent.EventType.CREATE =>
Some(Watcher.EventTypeCreate)
case DirectoryChangeEvent.EventType.MODIFY =>
Some(Watcher.EventTypeModify)
case DirectoryChangeEvent.EventType.DELETE =>
Some(Watcher.EventTypeDelete)
case DirectoryChangeEvent.EventType.OVERFLOW => None
}
object WatcherEvent {
/** Conversion form file system event to [[WatcherEvent]]
*
* @param event file system event
* @return watcher event
*/
def from(event: DirectoryChangeEvent): Option[WatcherEvent] =
EventType
.from(event.eventType())
.map(WatcherEvent(event.path(), _))
}
/** Object representing en error.
/** Conversion form file system event to [[Watcher.WatcherEvent]]
*
* @param exception an error
* @param event file system event
* @return watcher event
*/
case class WatcherError(exception: Exception)
def build(
root: Path,
eventCallback: WatcherEvent => Unit,
exceptionCallback: WatcherError => Unit
): WatcherAdapter =
new WatcherAdapter(root, eventCallback, exceptionCallback)
private def watcherEvent(
event: DirectoryChangeEvent
): Option[Watcher.WatcherEvent] =
eventType(event.eventType())
.map(Watcher.WatcherEvent(event.path(), _))
}

View File

@ -0,0 +1,13 @@
package org.enso.filewatcher
import java.nio.file.Path
class WatcherAdapterFactory extends WatcherFactory {
/** @inheritdoc */
override def build(
root: Path,
eventCallback: Watcher.WatcherEvent => Unit,
exceptionCallback: Watcher.WatcherError => Unit
): Watcher =
new WatcherAdapter(root, eventCallback, exceptionCallback)
}

View File

@ -0,0 +1,19 @@
package org.enso.filewatcher
import java.nio.file.Path
trait WatcherFactory {
/** Create a instance of [[Watcher]].
*
* @param root the directory to watch
* @param eventCallback the callback handling the file watcher events
* @param exceptionCallback the callback handling the file watcher errors
* @return a new instance of [[Watcher]]
*/
def build(
root: Path,
eventCallback: Watcher.WatcherEvent => Unit,
exceptionCallback: Watcher.WatcherError => Unit
): Watcher
}

View File

@ -13,8 +13,6 @@ import scala.util.Try
class WatcherAdapterSpec extends AnyFlatSpec with Matchers with RetrySpec {
import WatcherAdapter._
final val Timeout: FiniteDuration = 5.seconds
it should "get create events" taggedAs Retry in withWatcher {
@ -22,7 +20,7 @@ class WatcherAdapterSpec extends AnyFlatSpec with Matchers with RetrySpec {
val fileA = Paths.get(path.toString, "a.txt")
Files.createFile(fileA)
val event = events.poll(Timeout.length, Timeout.unit)
event shouldBe WatcherAdapter.WatcherEvent(fileA, EventTypeCreate)
event shouldBe Watcher.WatcherEvent(fileA, Watcher.EventTypeCreate)
}
it should "get delete events" taggedAs Retry in withWatcher {
@ -31,11 +29,11 @@ class WatcherAdapterSpec extends AnyFlatSpec with Matchers with RetrySpec {
Files.createFile(fileA)
val event1 = events.poll(Timeout.length, Timeout.unit)
event1 shouldBe WatcherEvent(fileA, EventTypeCreate)
event1 shouldBe Watcher.WatcherEvent(fileA, Watcher.EventTypeCreate)
Files.delete(fileA)
val event2 = events.poll(Timeout.length, Timeout.unit)
event2 shouldBe WatcherEvent(fileA, EventTypeDelete)
event2 shouldBe Watcher.WatcherEvent(fileA, Watcher.EventTypeDelete)
}
it should "get modify events" taggedAs Retry in withWatcher {
@ -44,11 +42,11 @@ class WatcherAdapterSpec extends AnyFlatSpec with Matchers with RetrySpec {
Files.createFile(fileA)
val event1 = events.poll(Timeout.length, Timeout.unit)
event1 shouldBe WatcherEvent(fileA, EventTypeCreate)
event1 shouldBe Watcher.WatcherEvent(fileA, Watcher.EventTypeCreate)
Files.write(fileA, "hello".getBytes())
val event2 = events.poll(Timeout.length, Timeout.unit)
event2 shouldBe WatcherEvent(fileA, EventTypeModify)
event2 shouldBe Watcher.WatcherEvent(fileA, Watcher.EventTypeModify)
}
it should "get events from subdirectories" taggedAs Retry in withWatcher {
@ -58,21 +56,21 @@ class WatcherAdapterSpec extends AnyFlatSpec with Matchers with RetrySpec {
Files.createDirectories(subdir)
val event1 = events.poll(Timeout.length, Timeout.unit)
event1 shouldBe WatcherEvent(subdir, EventTypeCreate)
event1 shouldBe Watcher.WatcherEvent(subdir, Watcher.EventTypeCreate)
Files.createFile(fileA)
val event2 = events.poll(Timeout.length, Timeout.unit)
event2 shouldBe WatcherEvent(fileA, EventTypeCreate)
event2 shouldBe Watcher.WatcherEvent(fileA, Watcher.EventTypeCreate)
}
def withWatcher(
test: (Path, LinkedBlockingQueue[WatcherEvent]) => Any
test: (Path, LinkedBlockingQueue[Watcher.WatcherEvent]) => Any
): Any = {
val lock = new Semaphore(0)
val executor = Executors.newSingleThreadExecutor()
val tmp = Files.createTempDirectory(null).toRealPath()
val queue = new LinkedBlockingQueue[WatcherEvent]()
val watcher = WatcherAdapter.build(tmp, queue.put, println(_))
val queue = new LinkedBlockingQueue[Watcher.WatcherEvent]()
val watcher = new WatcherAdapterFactory().build(tmp, queue.put, println(_))
executor.submit[Any] { () =>
lock.release()