Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

distage-testkit: Smart memoization group merging. #1072

Merged
merged 8 commits into from
May 22, 2020

Conversation

Caparow
Copy link
Contributor

@Caparow Caparow commented May 22, 2020

fix #1067

@codecov-commenter
Copy link

codecov-commenter commented May 22, 2020

Codecov Report

Merging #1072 into develop will increase coverage by 0.27%.
The diff coverage is 92.59%.

@@             Coverage Diff             @@
##           develop    #1072      +/-   ##
===========================================
+ Coverage    55.65%   55.93%   +0.27%     
===========================================
  Files          380      381       +1     
  Lines         6917     6940      +23     
  Branches       362      358       -4     
===========================================
+ Hits          3850     3882      +32     
+ Misses        3067     3058       -9     

@neko-kai
Copy link
Member

neko-kai commented May 22, 2020

A thought: we can limit the amount of logs for re-reading the same config files by memoizing config reads with the same testBaseName (for example using a mutable map and getOrElseUpdate)

@Caparow
Copy link
Contributor Author

Caparow commented May 22, 2020

A thought: we can limit the amount of logs for re-reading the same config files by memoizing config reads with the same testBaseName (for example using a mutable map and getOrElseUpdate)

A good idea!
Will do

@neko-kai
Copy link
Member

Another thought, we can replace testRunnerLogLevel with just a switch debugOutput: Boolean and set level Debug on true, Info on false, and force level to Debug if property DebugProperties.\izumi.distage.testkit.debug`is set to true. Then we can also remove the separatedebugLoggerand just log withIzLogger.debug`.

@Caparow
Copy link
Contributor Author

Caparow commented May 22, 2020

Another thought, we can replace testRunnerLogLevel with just a switch debugOutput: Boolean and set level Debug on true, Info on false, and force level to Debug if property DebugProperties.\izumi.distage.testkit.debug``is set to true. Then we can also remove the separatedebugLoggerand just log with`IzLogger.debug`.

But by this implementation we will lose control over the test log level. It's okay to have info level always, but sometimes we need to force warn level to get rid of the noise.

@neko-kai
Copy link
Member

@Caparow Hmm, actually earlyLogger is used just once - when loading config, and currently there's no way to disable config loader printing - afterwards logger settings are read from config. Hmm, we may alter the log levels of the debug messages themselves (promote Debug to Info) instead of chaging the logger. We may also have three values for debugOutput = Silent/Normal/Verbose

@Caparow
Copy link
Contributor Author

Caparow commented May 22, 2020

@Caparow Hmm, actually earlyLogger is used just once - when loading config, and currently there's no way to disable config loader printing - afterwards logger settings are read from config. Hmm, we may alter the log levels of the debug messages themselves (promote Debug to Info) instead of chaging the logger. We may also have three values for debugOutput = Silent/Normal/Verbose

You still can silence the config loader. Early logger uses testRunnerLogLevel. Also, a late logger is inherited from early logger. And there is no point to add new log level ADT (at least I don't know why we'll need this).

@Caparow Caparow force-pushed the feature/testkit-smart-envs-merge branch from 3adbc2a to 8039ff9 Compare May 22, 2020 18:55
@neko-kai neko-kai merged commit e9354b0 into develop May 22, 2020
@neko-kai neko-kai deleted the feature/testkit-smart-envs-merge branch May 22, 2020 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

distage-testkit: Smart memoization group merging
3 participants