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

bareos tools: reintegrate testfind binary #1176

Merged

Conversation

HediBenFraj
Copy link
Contributor

@HediBenFraj HediBenFraj commented Apr 28, 2022

Description

Reintegrated testfind into the build and fixed logic and compile errors.
Testfind is a tool that uses the same logic used by the file daemon before a backup.
we can use it to benchmark directories/files without launching a backup job.

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

General
  • PR name is meaningful
  • Purpose of the PR is understood
  • Separate commit for this PR in the CHANGELOG.md, PR number referenced is same
  • Commit descriptions are understandable and well formatted
  • If backport: add original PR number and target branch at top of this file: Backport of PR#000 to bareos-2x
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR
  • bareos-check-sources --since-merge does not report any problems
  • git status should not report modifications in the source tree after building and testing
Tests
  • Decision taken that a test is required (if not, then remove this paragraph)
  • The choice of the type of test (unit test or systemtest) is reasonable
  • Testname matches exactly what is being tested
  • On a fail, output of the test leads quickly to the origin of the fault

@pstorz pstorz self-assigned this Apr 29, 2022
@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch 2 times, most recently from c66ad31 to af445ca Compare May 5, 2022 14:28
@pstorz pstorz changed the title Dev/hedi ben fraj/master/build testfind build testfind May 11, 2022
@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch from d6f6b1b to 5f45097 Compare May 12, 2022 14:20
@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch from 2eed37c to a95806b Compare June 23, 2022 10:23
@HediBenFraj HediBenFraj marked this pull request as ready for review June 24, 2022 09:03
@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch from a95806b to f895291 Compare June 27, 2022 03:05
@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch 12 times, most recently from f36d2e4 to 9bb1a72 Compare July 21, 2022 11:48
@pstorz pstorz self-requested a review July 21, 2022 12:15
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Running the code with sanitizers, it shows the following:
/core/src/dird/testfind.cc:104:58: runtime error: member call on null pointer of type 'struct ConfigurationParser'

@HediBenFraj HediBenFraj marked this pull request as draft July 28, 2022 09:57
@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch from 9bb1a72 to 1905681 Compare July 29, 2022 13:05
@HediBenFraj HediBenFraj marked this pull request as ready for review August 1, 2022 08:49
@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch 2 times, most recently from 7f1c486 to e22c7dc Compare August 5, 2022 10:04
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Running the systemtest for testfind, an error is reported but still the test passes:

389: Test command: /home/pstorz/git/PR-REVIEW/b/systemtests/tests/testfind/testrunner
389: Test timeout computed to be: 300
389: Expected line "Max path length: 80" was not found in log file "/home/pstorz/git/PR-REVIEW/b/systemtests/tests/testfind/tmp/testfind.out".
389: -->Max path length is incorrect.
1/1 Test #389: system:testfind ..................   Passed    0.10 sec

The following tests passed:
        system:testfind

100% tests passed, 0 tests failed out of 1

@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch from e22c7dc to de5f896 Compare August 8, 2022 19:48
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Please answer the questions that have been asked in the last review.

core/src/dird/testfind.cc Outdated Show resolved Hide resolved
core/src/dird/testfind.cc Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
core/src/dird/testfind.cc Outdated Show resolved Hide resolved
@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch from de5f896 to b7e30c8 Compare August 16, 2022 12:13
@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch 2 times, most recently from 6e0f45f to 7d36f32 Compare August 19, 2022 08:54
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Please check the comments.

core/src/dird/testfind.cc Outdated Show resolved Hide resolved
core/src/dird/testfind.cc Outdated Show resolved Hide resolved
core/src/dird/testfind.cc Show resolved Hide resolved
core/src/dird/testfind.cc Outdated Show resolved Hide resolved
core/src/dird/testfind.cc Outdated Show resolved Hide resolved
core/src/lib/parse_conf.h Outdated Show resolved Hide resolved
@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch from 7d36f32 to 9080bee Compare August 23, 2022 12:16
@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch from 9080bee to 51a3844 Compare August 23, 2022 13:51
@HediBenFraj HediBenFraj changed the title build testfind Reintegrate testfind Aug 26, 2022
@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch from 5817356 to e9938e8 Compare August 26, 2022 11:00
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

We have a typo in commit 224eeff: wirndows
Please fix that one.

Also the CHANGELOG.md entry is not exactly like the PR name is.

CHANGELOG.md Outdated
@@ -42,6 +42,7 @@ and since Bareos version 20 this project adheres to [Semantic Versioning](https:
- build: Add support for SLE_15_SP4 [PR #1205]
- libcloud plugin: allow to configure the storage provider [PR #1226]
- core/platform: Adding Bareos firewalld service xml files [PR #1237]
- bareos tools: reintegrated testfind binary [PR #1176]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- bareos tools: reintegrated testfind binary [PR #1176]
- bareos tools: reintegrate testfind binary [PR #1176]

@pstorz pstorz self-requested a review August 29, 2022 09:31
Copy link
Member

@pstorz pstorz left a comment

Choose a reason for hiding this comment

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

Also added the bareos-check-sources changes as one commit

@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch 2 times, most recently from 1ff580d to 098c1ec Compare September 1, 2022 08:59
@HediBenFraj HediBenFraj force-pushed the dev/HediBenFraj/master/build-testfind branch from 098c1ec to bb3f749 Compare September 1, 2022 10:24
@pstorz pstorz changed the title Reintegrate testfind bareos tools: reintegrate testfind binary Sep 1, 2022
@pstorz pstorz merged commit 61a6110 into bareos:master Sep 1, 2022
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.

None yet

2 participants