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.

Sign up for GitHub

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

Jump to bottom

More robust unit tests: Strict checking of output as expected causes many false positives #1539

Closed
aryoda opened this issue Sep 22, 2023 · 4 comments · Fixed by #1544
Closed

More robust unit tests: Strict checking of output as expected causes many false positives #1539

aryoda opened this issue Sep 22, 2023 · 4 comments · Fixed by #1544
Assignees
@buhtz
Labels
Discussion decision or consensus needed
Milestone
1.4.1 (upcoming r...

Comments

@aryoda
Copy link
Contributor

aryoda commented Sep 22, 2023

Background

Several unit tests do check the captures console output ("actual") against the expected output.

Impact

This is not very robust (causes many false positives due to unexpected warnings mainly caused by different external setup constellations).

Current implementation

Currently well known "false positive" warnings are filtered out from the actual console output:

https://github.com/bit-team/backintime/blob/dev/common/test/test_backintime.py#L143-L150

Proposals

Either

  • filter out all warnings in a generic way
  • or switch from console output checking to another method (eg. comparing the created snapshot instead)

References

See also the side discussion in this PR which added another tolerated warning:

#1537 (comment)
@aryoda aryoda added the Discussion decision or consensus needed label Sep 22, 2023
@aryoda
Copy link
Contributor Author

aryoda commented Sep 22, 2023

@buhtz As a first step I think we should really filter out all warnings (as you have proposed).

@buhtz
Copy link
Member

buhtz commented Sep 22, 2023

@buhtz As a first step I think we should really filter out all warnings (as you have proposed).

OK, that is a compromise I can live with. After 1.4.1 or before?

@aryoda
Copy link
Contributor Author

aryoda commented Sep 22, 2023

@buhtz As a first step I think we should really filter out all warnings (as you have proposed).

OK, that is a compromise I can live with. After 1.4.1 or before?

We could do it for v1.4.1 if another warning needed to be added to the exclusion list, otherwise directly after the v1.4.1 release.

And for refactoring and improving the unit tests I have no idea how a realistic schedule could look alike.

@buhtz
Copy link
Member

buhtz commented Sep 22, 2023

We could do it for v1.4.1

OK, I could take hand on that in the next two weeks.

And for refactoring and improving the unit tests I have no idea how a realistic schedule could look alike.

This is IMHO an ongoing task. We will discover problems like this all along the road.

@buhtz buhtz self-assigned this Sep 22, 2023
@buhtz buhtz added this to the 1.4.1 (upcoming release) milestone Sep 22, 2023
@buhtz buhtz mentioned this issue Sep 25, 2023
Merged
buhtz added a commit that referenced this issue Sep 25, 2023
@buhtz
build: Ignore WARNING in specific unittest
5efb5d6 
The test TestBackInTime::test_local_snapshot_is_successful() now do ignore all log entries beginning with Warning or WARNING.

Also slightly improved SSH Server related testing code in generic.py and added a FAQ entry for it.

Fix #1539
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@buhtz buhtz

Labels
Discussion decision or consensus needed
Projects
None yet
Milestone
1.4.1 (upcoming release)
Development

Successfully merging a pull request may close this issue.

build: Ignore WARNING in specific unittest buhtz/backintime
2 participants
@aryoda @buhtz