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

DAOS-13771 control: Unit test TestSupport_getRunningConf() fix #12456

Merged
merged 6 commits into from
Jul 6, 2023
Merged

DAOS-13771 control: Unit test TestSupport_getRunningConf() fix #12456

merged 6 commits into from
Jul 6, 2023

Conversation

ravalsam
Copy link
Contributor

@ravalsam ravalsam commented Jun 20, 2023 •
edited
Loading

Removing the unit tests which required fake engine to start, And this test case already part of functional test.

Quick-build: true
Skip-func-test: true

Required-githooks: true

Before requesting gatekeeper:

  • Two review approvals and any prior change requests have been resolved.
  • Testing is complete and all tests passed or there is a reason documented in the PR why it should be force landed and forced-landing tag is set.
  • Features: (or Test-tag*) commit pragma was used or there is a reason documented that there are no appropriate tags for this PR.
  • Commit messages follows the guidelines outlined here.
  • Any tests skipped by the ticket being addressed have been run and passed in the PR.

Gatekeeper:

  • You are the appropriate gatekeeper to be landing the patch.
  • The PR has 2 reviews by people familiar with the code, including appropriate watchers.
  • Githooks were used. If not, request that user install them and check copyright dates.
  • Checkpatch issues are resolved. Pay particular attention to ones that will show up on future PRs.
  • All builds have passed. Check non-required builds for any new compiler warnings.
  • Sufficient testing is done. Check feature pragmas and test tags and that tests skipped for the ticket are run and now pass with the changes.
  • If applicable, the PR has addressed any potential version compatibility issues.
  • Check the target branch. If it is master branch, should the PR go to a feature branch? If it is a release branch, does it have merge approval in the JIRA ticket.
  • Extra checks if forced landing is requested
    • Review comments are sufficiently resolved, particularly by prior reviewers that requested changes.
    • No new NLT or valgrind warnings. Check the classic view.
    • Quick-build or Quick-functional is not used.
  • Fix the commit message upon landing. Check the standard here. Edit it to create a single commit. If necessary, ask submitter for a new summary.

@ravalsam ravalsam requested a review from a team as a code owner June 20, 2023 18:17
@ravalsam ravalsam requested review from mjmac and removed request for a team June 20, 2023 18:17
@github-actions
Copy link

github-actions bot commented Jun 20, 2023 •
edited
Loading

Bug-tracker data:
Ticket title is 'Intermittent unit test failure in TestSupport_getRunningConf()'
Status is 'In Review'
Labels: 'triaged'
https://daosio.atlassian.net/browse/DAOS-13771

@daosbuild1
Copy link
Collaborator

Test stage Unit Test with memcheck on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-12456/1/testReport/

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev with memcheck on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-12456/1/testReport/

@ravalsam ravalsam force-pushed the samirrav/unittest/DAOS-13771 branch from b8de227 to b0a33c6 Compare June 20, 2023 19:44
@ashleypittman
Copy link
Contributor

Bug-tracker data: Ticket title is 'Intermittent unit test failure in TestSupport_getRunningConf()' Status is 'In Progress' Errors are Title of PR is too long https://daosio.atlassian.net/browse/DAOS-13771

Can you re-work the title of the PR to fit within the recommended character length.

@ravalsam ravalsam changed the title DAOS-13771 control: Intermittent unit test failure in TestSupport_getRunningConf() DAOS-13771 control: Unit test TestSupport_getRunningConf() fix Jun 20, 2023
@daosbuild1
Copy link
Collaborator

Test stage Unit Test with memcheck on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-12456/2/testReport/

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev with memcheck on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-12456/2/testReport/

@daosbuild1
Copy link
Collaborator

Test stage Unit Test with memcheck on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-12456/3/testReport/

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev with memcheck on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-12456/3/testReport/

@daosbuild1
Copy link
Collaborator

Test stage Unit Test with memcheck on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-12456/4/testReport/

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev with memcheck on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-12456/4/testReport/

@daosbuild1
Copy link
Collaborator

Test stage Unit Test on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-12456/5/testReport/

@daosbuild1
Copy link
Collaborator

Test stage Unit Test with memcheck on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-12456/5/testReport/

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev with memcheck on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-12456/5/testReport/

@ravalsam
DAOS-13771 control: Unit test TestSupport_getRunningConf() fix.
7798a52 
Testing the unit test failure in CI.

Quick-build: true
Skip-func-test: true

Required-githooks: true

Signed-off-by: Samir Raval <samir.raval@intel.com>
@ravalsam ravalsam force-pushed the samirrav/unittest/DAOS-13771 branch from b0a33c6 to 7798a52 Compare June 21, 2023 17:34
@daosbuild1
Copy link
Collaborator

Test stage Build RPM on Leap 15.4 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-12456/6/execution/node/209/log

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-12456/6/execution/node/206/log

@daosbuild1
Copy link
Collaborator

Test stage Build DEB on Ubuntu 20.04 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-12456/6/execution/node/231/log

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on Leap 15.4 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-12456/7/execution/node/213/log

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-12456/7/execution/node/240/log

@daosbuild1
Copy link
Collaborator

Test stage Build DEB on Ubuntu 20.04 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-12456/7/execution/node/197/log

daosbuild1
daosbuild1 reviewed Jun 21, 2023
View reviewed changes
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on Leap 15.4 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-12456/8/execution/node/325/log

@daosbuild1
Copy link
Collaborator

Test stage Build RPM on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-12456/8/execution/node/349/log

@ravalsam
Merge branch 'master' into samirrav/unittest/DAOS-13771
e05f5bb 
Required-githooks: true

Quick-build: true
Skip-func-test: true

Signed-off-by: Samir Raval <samir.raval@intel.com>
@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev with memcheck on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-12456/29/testReport/

@ravalsam
Removing the test code which required the fake engine to start.
c43198f 
This testing is covered as part of functional tests.

Quick-build: true
Skip-func-test: true

Required-githooks: true

Signed-off-by: Samir Raval <samir.raval@intel.com>
@ravalsam ravalsam dismissed stale reviews from kjacque and tanabarr via c43198f June 27, 2023 17:07
@daosbuild1
Copy link
Collaborator

Test stage Unit Test with memcheck on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-12456/30/testReport/

@daosbuild1
Copy link
Collaborator

Test stage Unit Test bdev with memcheck on EL 8 completed with status UNSTABLE. https://build.hpdd.intel.com/job/daos-stack/job/daos//view/change-requests/job/PR-12456/30/testReport/

@ravalsam ravalsam requested a review from a team June 30, 2023 15:48
@ravalsam ravalsam added the forced-landing The PR has known failures or has intentionally reduced testing, but should still be landed. label Jun 30, 2023
@ashleypittman
Copy link
Contributor

This PR has 188 valgrind reports which I don't think are known, is that expected?

@ashleypittman
Copy link
Contributor

In fact looking at the test results almost every build of this PR has the same 188 errors, I don't think this is ready for landing yet.

mchaarawi
mchaarawi reviewed Jun 30, 2023
View reviewed changes
Copy link
Contributor

@mchaarawi mchaarawi left a comment

Choose a reason for hiding this comment

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

non of the valgrind issues and unit test failures seem to be on master or seen elsewhere. please re-request gatekeeper when this is ready.

@ravalsam
Copy link
Contributor Author

non of the valgrind issues and unit test failures seem to be on master or seen elsewhere. please re-request gatekeeper when this is ready.

Some of the tests from go code is removed. Not sure how this related to other unit test failure.
I will check.

daosbuild1
daosbuild1 reviewed Jun 30, 2023
View reviewed changes
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@ravalsam
Copy link
Contributor Author

I rebase the master code and ran default tests without any tag and all unit test worked fine. I am not sure if Quick-build: true is making those failures or not.
But this PR seems ready to land as all unit test passed.

@ravalsam ravalsam requested a review from a team June 30, 2023 17:39
@daltonbohning
Copy link
Contributor

I rebase the master code and ran default tests without any tag and all unit test worked fine. I am not sure if Quick-build: true is making those failures or not. But this PR seems ready to land as all unit test passed.

Quick-build: true does have problems. I'm not sure if it should cause the unit tests, but I wouldn't be surprised. But either way, Quick-build: true shouldn't be used for landing anyway

@ashleypittman
Copy link
Contributor

Can you merge this with latest master and then re-push without any commit tags please?

daosbuild1
daosbuild1 reviewed Jul 5, 2023
View reviewed changes
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@ravalsam ravalsam removed the forced-landing The PR has known failures or has intentionally reduced testing, but should still be landed. label Jul 5, 2023
@ravalsam ravalsam requested a review from mchaarawi July 5, 2023 23:04
@ravalsam
Copy link
Contributor Author

ravalsam commented Jul 6, 2023

Can you merge this with latest master and then re-push without any commit tags please?

Merged with master and all checks looks OK. Please merge this.

@mchaarawi mchaarawi merged commit dabc47e into master Jul 6, 2023
@mchaarawi mchaarawi deleted the samirrav/unittest/DAOS-13771 branch July 6, 2023 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daosbuild1 daosbuild1 daosbuild1 left review comments

@knard-intel knard-intel knard-intel approved these changes

@kjacque kjacque kjacque approved these changes

@mjmac mjmac Awaiting requested review from mjmac mjmac is a code owner automatically assigned from daos-stack/go-watchers

@tanabarr tanabarr Awaiting requested review from tanabarr

@mchaarawi mchaarawi Awaiting requested review from mchaarawi

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@ravalsam @daosbuild1 @ashleypittman @daltonbohning @tanabarr @mchaarawi @kjacque @knard-intel