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

Add a smoke test for `spine-logging #800

Merged
merged 3 commits into from
Aug 4, 2023
Merged

Conversation

yevhenii-nadtochii
Copy link
Contributor

@yevhenii-nadtochii yevhenii-nadtochii commented Aug 3, 2023

This PR adds a smoke test for spine-logging to make sure that the logging is working in concrete modules.

base module is the first one where this test is added. Further, it will be added to more modules.

Spine dependency object will also be updated in config when this PR is approved.

@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

Merging #800 (4e5378e) into master (0a538f5) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #800   +/-   ##
=========================================
  Coverage     74.73%   74.73%           
  Complexity     1143     1143           
=========================================
  Files           180      180           
  Lines          4203     4203           
  Branches        335      335           
=========================================
  Hits           3141     3141           
  Misses          934      934           
  Partials        128      128           

@yevhenii-nadtochii yevhenii-nadtochii marked this pull request as ready for review August 3, 2023 15:17
@yevhenii-nadtochii
Copy link
Contributor Author

@alexander-yevsyukov PTAL

Copy link
Contributor

@alexander-yevsyukov alexander-yevsyukov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see my comment.

*
* The abstract base is provided by `Spine.Logging.smokeTest` dependency.
*/
internal class LoggingSmokeTest : AbstractLoggingSmokeTest()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look, this is too abstract (pun intended). We need to test that logging in this project does something useful for this library.

Copy link
Contributor

@alexander-yevsyukov alexander-yevsyukov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Let's discuss the abstract test matter when @armiol is back.

@yevhenii-nadtochii yevhenii-nadtochii added this pull request to the merge queue Aug 4, 2023
Merged via the queue into master with commit 2649881 Aug 4, 2023
7 checks passed
@yevhenii-nadtochii yevhenii-nadtochii deleted the update-config branch August 4, 2023 11:58
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.

2 participants