Skip to content

Commit

Permalink
#236: fixes issue of wrongly selected/deselected filters
Browse files Browse the repository at this point in the history
  • Loading branch information
rladstaetter committed Jul 24, 2024
1 parent bac1103 commit a72d713
Show file tree
Hide file tree
Showing 30 changed files with 219 additions and 116 deletions.
3 changes: 2 additions & 1 deletion app-tests/src/test/resources/app/logorrr/SimpleLog-2.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
FINEST
INFO
WARNING
SEVERE
SEVERE
not classified
10 changes: 5 additions & 5 deletions app-tests/src/test/scala/app/logorrr/TestFiles.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ object TestFiles {

val baseDir: Path = Paths.get("src/test/resources/app/logorrr/")

val simpleLog0: FileId = FileId(baseDir.resolve("SimpleLog-0.txt"))
val simpleLog1: FileId = FileId(baseDir.resolve("SimpleLog-1.txt"))
val simpleLog2: FileId = FileId(baseDir.resolve("SimpleLog-2.txt"))
val simpleLog3: FileId = FileId(baseDir.resolve("SimpleLog-3.txt"))
val simpleLog4: FileId = FileId(baseDir.resolve("SimpleLog-4.txt"))
val simpleLog0: FileId = FileId(baseDir.resolve("SimpleLog-0.txt")) // 4 entries
val simpleLog1: FileId = FileId(baseDir.resolve("SimpleLog-1.txt")) // 100 entries
val simpleLog2: FileId = FileId(baseDir.resolve("SimpleLog-2.txt")) // 5 entries
val simpleLog3: FileId = FileId(baseDir.resolve("SimpleLog-3.txt")) // 4 entries
val simpleLog4: FileId = FileId(baseDir.resolve("SimpleLog-4.txt")) // 4 entries

val zipFileContaining10Files: FileId = FileId(baseDir.resolve("zip-containing-10-files.zip"))

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package app.logorrr.issues

import app.logorrr.TestFiles
import app.logorrr.model.LogFileSettings
import app.logorrr.steps.CheckTabPaneActions
import app.logorrr.usecases.MultipleFileApplicationTest
import app.logorrr.views.search.FilterButton
import javafx.scene.control.ToggleButton
import org.junit.jupiter.api.Test
import org.testfx.api.FxAssert

import java.util.function.Predicate

/**
* https://github.com/rladstaetter/LogoRRR/issues/236
*
* Shows that the default filters are used for a file after the first file was opened and the filter selection
* was changed.
* */
class Issue236OpenMultipleTabsAndChooseDefaultFilterTest
extends MultipleFileApplicationTest(TestFiles.seq)
with CheckTabPaneActions {

@Test def testIssue236(): Unit = {
// open first file
openFile(TestFiles.simpleLog0)

// change filters to a non default configuration
val firstFilterTab1 = FilterButton.uiNode(TestFiles.simpleLog0, LogFileSettings.DefaultFilters.head)
waitAndClickVisibleItem(firstFilterTab1)

// check that the toggle button is deselected
FxAssert.verifyThat(lookup(firstFilterTab1.ref), new Predicate[ToggleButton] {
override def test(t: ToggleButton): Boolean = !t.isSelected
})

// open second file
openFile(TestFiles.simpleLog1)

// test that second file has the default filter configuration
val firstFilterTab2 = FilterButton.uiNode(TestFiles.simpleLog1, LogFileSettings.DefaultFilters.head)
FxAssert.verifyThat(lookup(firstFilterTab2.ref), new Predicate[ToggleButton] {
override def test(t: ToggleButton): Boolean = t.isSelected
})
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package app.logorrr.services.file

/**
* Don't provide any FileId, for tests which don't need any file operations
*/
class EmptyFileIdService extends MockFileIdService(Seq())

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,11 @@ import app.logorrr.io.FileId
*
* @param files which this service is returning
*/
class MockFileService(files: Seq[FileId]) extends FileService {
class MockFileIdService(files: Seq[FileId]) extends FileIdService {

private val it = files.iterator

override def openFile: Option[FileId] = {
override def provideFileId: Option[FileId] = {
if (it.hasNext) {
Option(it.next())
} else None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@ import app.logorrr.io.FileId
*
* @param fileId file reference to open
*/
class SingleFileService(fileId: FileId) extends MockFileService(Seq(fileId))
class SingleFileIdService(fileId: FileId) extends MockFileIdService(Seq(fileId))
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ trait FileMenuActions extends VisibleItemActions {

def quitApplication(): Unit = {
waitAndClickVisibleItem(UiNodes.FileMenu)
waitAndClickVisibleItem(UiNodes.FileMenuQuitApplication)
waitAndClickVisibleItem(UiNodes.FileMenuCloseApplication)
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package app.logorrr.usecases
import app.logorrr.conf.Settings
import app.logorrr.io.FileId
import app.logorrr.services.LogoRRRServices
import app.logorrr.services.file.MockFileService
import app.logorrr.services.file.MockFileIdService
import app.logorrr.services.hostservices.MockHostServices
import app.logorrr.steps.{CanStartApplication, VisibleItemActions}

Expand All @@ -19,7 +19,7 @@ class MultipleFileApplicationTest(val files: Seq[FileId])

lazy val services: LogoRRRServices = LogoRRRServices(Settings.Default
, new MockHostServices
, new MockFileService(files)
, new MockFileIdService(files)
, isUnderTest = true)

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package app.logorrr.usecases
import app.logorrr.conf.Settings
import app.logorrr.io.FileId
import app.logorrr.services.LogoRRRServices
import app.logorrr.services.file.SingleFileService
import app.logorrr.services.file.SingleFileIdService
import app.logorrr.services.hostservices.MockHostServices
import app.logorrr.steps.CanStartApplication

Expand All @@ -16,7 +16,7 @@ class SingleFileApplicationTest(val fileId: FileId)

final def services: LogoRRRServices = LogoRRRServices(Settings.Default
, new MockHostServices
, new SingleFileService(fileId)
, new SingleFileIdService(fileId)
, isUnderTest = true)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package app.logorrr.usecases

import app.logorrr.conf.Settings
import app.logorrr.services.LogoRRRServices
import app.logorrr.services.file.EmptyFileService
import app.logorrr.services.file.EmptyFileIdService
import app.logorrr.services.hostservices.MockHostServices
import app.logorrr.steps.CanStartApplication

Expand All @@ -16,7 +16,7 @@ class StartEmptyApplicationTest

final def services: LogoRRRServices = LogoRRRServices(Settings.Default
, new MockHostServices
, new EmptyFileService
, new EmptyFileIdService
, isUnderTest = true)


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package app.logorrr.usecases.about

import app.logorrr.conf.Settings
import app.logorrr.services.LogoRRRServices
import app.logorrr.services.file.EmptyFileService
import app.logorrr.services.file.EmptyFileIdService
import app.logorrr.services.hostservices.MockHostServices
import app.logorrr.steps.{CanStartApplication, VisibleItemActions}
import app.logorrr.usecases.TestFxBaseApplicationTest
Expand All @@ -21,7 +21,7 @@ class ShowAboutDialogTest extends TestFxBaseApplicationTest
final def services: LogoRRRServices = {
LogoRRRServices(Settings.Default
, mockHostServices
, new EmptyFileService
, new EmptyFileIdService
, isUnderTest = true)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package app.logorrr.usecases.search
import app.logorrr.TestFiles
import app.logorrr.model.LogFileSettings
import app.logorrr.usecases.SingleFileApplicationTest
import app.logorrr.views.search.FilterButton
import app.logorrr.views.search.{Filter, FilterButton, UnclassifiedFilter}
import app.logorrr.views.text.LogTextView
import org.junit.jupiter.api.Test

Expand All @@ -13,22 +13,46 @@ import org.junit.jupiter.api.Test
class SelectDefaultFilterTest extends SingleFileApplicationTest(TestFiles.simpleLog2) {

@Test def selectSpecificFilter(): Unit = {
// file has 5 entries - each line consisting of one entry corresponding to
// the default filters, like 'FINEST', 'INFO, ' WARNING', 'SEVERE'
// and a 'unclassified' line
openFile(fileId)

LogFileSettings.DefaultFilters.foreach {
f =>
LogFileSettings.DefaultFilters.foreach {
ff => waitAndClickVisibleItem(FilterButton.uiNode(fileId, ff)) // disable all filters
}
waitAndClickVisibleItem(FilterButton.uiNode(fileId, f)) // enable one specific filer

val res = lookup(LogTextView.uiNode(fileId).ref).query[LogTextView]
assert(res.getItems.size() == 1)
waitAndClickVisibleItem(FilterButton.uiNode(fileId, f)) // enable disable specific filer

LogFileSettings.DefaultFilters.foreach {
ff => waitAndClickVisibleItem(FilterButton.uiNode(fileId, ff)) // enable all filters
}
// deselect all filters except unclassified
clickFilters(LogFileSettings.DefaultFilters)

// now, only 'unclassified' filter is active. since there are no unclassified
// entries available, the number of displayed log entries is one
checkNumberOfShownElements(1)

// select one specific filter- now two lines are shown in total
waitAndClickVisibleItem(FilterButton.uiNode(fileId, f))
checkNumberOfShownElements(2)

// deselect filter again
waitAndClickVisibleItem(FilterButton.uiNode(fileId, f))
clickFilters(LogFileSettings.DefaultFilters)
}

// finally, deselect all filters
clickFilters(LogFileSettings.DefaultFilters)
// one entry is shown (unclassified)
checkNumberOfShownElements(1)

// deselect unclassified filter works as well
clickFilters(Seq(UnclassifiedFilter(LogFileSettings.DefaultFilters.toSet)))
checkNumberOfShownElements(0)
}

def checkNumberOfShownElements(expectedElements: Int): Unit = {
assert(lookup(LogTextView.uiNode(fileId).ref).query[LogTextView].getItems.size() == expectedElements)
}

def clickFilters(filters: Seq[Filter]): Unit = {
filters.foreach {
ff => waitAndClickVisibleItem(FilterButton.uiNode(fileId, ff)) // enable all filters
}
}

Expand Down
6 changes: 3 additions & 3 deletions app/src/main/scala/app/logorrr/LogoRRRApp.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import app.logorrr.conf.{LogoRRRGlobals, SettingsIO}
import app.logorrr.io.FilePaths
import app.logorrr.meta.AppMeta
import app.logorrr.services.LogoRRRServices
import app.logorrr.services.file.DefaultFileService
import app.logorrr.services.file.DefaultFileIdService
import app.logorrr.services.hostservices.{MacNativeHostService, NativeHostServices}
import app.logorrr.util.{CanLog, JfxUtils, OsUtil}
import app.logorrr.views.main.{LogoRRRMain, LogoRRRStage}
Expand Down Expand Up @@ -36,7 +36,7 @@ object LogoRRRApp extends CanLog {
Application.setUserAgentStylesheet("/app/logorrr/LogoRRR.css")

LogoRRRGlobals.set(services.settings, services.hostServices)
val logoRRRMain = new LogoRRRMain(JfxUtils.closeStage(stage), services.fileOpenService, services.isUnderTest)
val logoRRRMain = new LogoRRRMain(JfxUtils.closeStage(stage), services.fileIdService, services.isUnderTest)
LogoRRRStage.init(stage, logoRRRMain)

logInfo(s"Started ${AppMeta.fullAppNameWithVersion} in '${Paths.get("").toAbsolutePath.toString}'")
Expand All @@ -58,7 +58,7 @@ class LogoRRRApp extends javafx.application.Application with CanLog {
val services = logorrr.services.LogoRRRServices(
SettingsIO.fromFile(FilePaths.settingsFilePath)
, hostServices
, new DefaultFileService(() => stage.getScene.getWindow)
, new DefaultFileIdService(() => stage.getScene.getWindow)
, isUnderTest = false)

LogoRRRApp.start(stage, services)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ object MutLogFileSettings {

def apply(logFileSettings: LogFileSettings): MutLogFileSettings = {
val s = new MutLogFileSettings
s.setFileId(logFileSettings.fileId)
s.setSelectedLineNumber(logFileSettings.selectedLineNumber)
s.setFontSize(logFileSettings.fontSize)
s.setBlockSettings(logFileSettings.blockSettings)
s.setFileId(logFileSettings.fileId)
s.firstOpenedProperty.set(logFileSettings.firstOpened)
s.setDividerPosition(logFileSettings.dividerPosition)
s.setFilters(logFileSettings.filters)
Expand Down
19 changes: 14 additions & 5 deletions app/src/main/scala/app/logorrr/model/LogFileSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,12 @@ package app.logorrr.model

import app.logorrr.conf.BlockSettings
import app.logorrr.io.FileId
import app.logorrr.util.CanLog
import app.logorrr.views.search.Filter
import javafx.scene.paint.Color
import pureconfig.generic.semiauto.{deriveReader, deriveWriter}
import pureconfig.{ConfigReader, ConfigWriter}

import java.nio.file.{Path, Paths}
import java.nio.file.Path
import java.time.Instant

object LogFileSettings {
Expand All @@ -27,7 +26,6 @@ object LogFileSettings {
private val InfoFilter: Filter = new Filter("INFO", Color.GREEN, true)
private val WarningFilter: Filter = new Filter("WARNING", Color.ORANGE, true)
private val SevereFilter: Filter = new Filter("SEVERE", Color.RED, true)

val DefaultFilters: Seq[Filter] = Seq(FinestFilter, InfoFilter, WarningFilter, SevereFilter)

private val DefaultFontSize = 12
Expand Down Expand Up @@ -57,6 +55,17 @@ object LogFileSettings {
*
* Filters define which keywords are relevant for this given log file.
*
* @param fileId wraps file reference
* @param selectedLineNumber which line number was selected
* @param firstOpened used to sort log files in tabs
* @param dividerPosition position of divider for this view
* @param fontSize font size to use
* @param filters filters which should be applied
* @param blockSettings settings for the left view
* @param someLogEntryInstantFormat used timestamp format
* @param autoScroll true if 'follow mode' is active
* @param firstVisibleTextCellIndex which index is the first visible on the screen (depending on resolution, window size ...)
* @param lastVisibleTextCellIndex which index is the last visible on the screen (depending on resolution, window size ...)
*/
case class LogFileSettings(fileId: FileId
, selectedLineNumber: Int
Expand All @@ -68,8 +77,8 @@ case class LogFileSettings(fileId: FileId
, someLogEntryInstantFormat: Option[LogEntryInstantFormat]
, autoScroll: Boolean
, firstVisibleTextCellIndex: Int
, lastVisibleTextCellIndex: Int) extends CanLog {
, lastVisibleTextCellIndex: Int) {

val path: Path = Paths.get(fileId.value).toAbsolutePath
val path: Path = fileId.asPath.toAbsolutePath

}
15 changes: 12 additions & 3 deletions app/src/main/scala/app/logorrr/services/LogoRRRServices.scala
Original file line number Diff line number Diff line change
@@ -1,10 +1,19 @@
package app.logorrr.services

import app.logorrr.conf.Settings
import app.logorrr.services.file.FileService
import app.logorrr.services.file.FileIdService
import app.logorrr.services.hostservices.LogoRRRHostServices

/**
* Groups settings, acts as a kind of service provider interface
*
* @param settings application settings
* @param hostServices services which need native interfaces which in turn need special capabilities which
* have to be declared on packaging
* @param fileIdService service to lookup file ids, also needed because of security/packaging
* @param isUnderTest helper flag which tells the application it runs in test mode
*/
case class LogoRRRServices(settings: Settings
, hostServices: LogoRRRHostServices
, fileOpenService: FileService
, hostServices: LogoRRRHostServices
, fileIdService: FileIdService
, isUnderTest: Boolean)
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ import javafx.stage.Window
*
* @param getWindow function to determine current window
*/
class DefaultFileService(getWindow: () => Window) extends FileService {
class DefaultFileIdService(getWindow: () => Window) extends FileIdService {

override def openFile: Option[FileId] = {
override def provideFileId: Option[FileId] = {
new LogoRRRFileChooser("Open log file").performShowAndWait(getWindow())
}

Expand Down
13 changes: 13 additions & 0 deletions app/src/main/scala/app/logorrr/services/file/FileIdService.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package app.logorrr.services.file

import app.logorrr.io.FileId

/**
* Trait to inject either Test FileId Services or query the native dialog
*/
trait FileIdService {

/** returns either None or a valid FileId */
def provideFileId: Option[FileId]

}
Loading

0 comments on commit a72d713

Please sign in to comment.