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

common: Unified pmem tests #5740

Merged
merged 4 commits into from
Jun 30, 2023
Merged

common: Unified pmem tests #5740

merged 4 commits into from
Jun 30, 2023

Conversation

grom72
Copy link
Contributor

@grom72 grom72 commented Jun 19, 2023

The second PR (out of two) unifies how Bash and Python main test scripts can be called from GHA.

In this PR:

  • build names are harmonized for Bash and Python tests execution
    -- release -> nondebug
    -- static-debug -> static_debug and static-nondebug -> static_nondebug (underscore _ instead of dash -)
  • single file for PMem Valgrind tests has been created - pmem_valgrind.yml, instead of two separate files for Python and Bash
  • long test description (pmem_long.yml) has been simplified

Requires:

This PR is a pre-work for #5702.


This change is Reviewable

@grom72 grom72 requested a review from janekmi June 19, 2023 06:49
@grom72 grom72 added Priority: 3 medium QA: CI .github/ and utils/ related to automated testing labels Jun 19, 2023
@grom72 grom72 added this to the 2.0.0 milestone Jun 19, 2023
@grom72 grom72 force-pushed the unified-pmem-tests branch 2 times, most recently from 064eb68 to 478199f Compare June 23, 2023 10:34
Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Respect. This is one huge step forward. :lgtm:

Reviewed 4 of 10 files at r4, 7 of 12 files at r5, 26 of 29 files at r6, 3 of 3 files at r7, 1 of 1 files at r8, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @grom72)


.github/workflows/pmem_check.yml line 20 at r8 (raw file):

                 'check TEST_BUILD=nondebug',
                 'check TEST_BUILD=static-debug',
                 'check TEST_BUILD=static-nondebug',

Wow. Another variation? Did it work at all?


.github/workflows/pmem_valgrind.yml line 21 at r8 (raw file):

      matrix:
        config: ['drd', 'pmemcheck', 'memcheck', 'helgrind']
        build: ['debug', 'release', 'static_debug', 'static_release']

How has it worked previously? AFAIK there is no "release" PMDK release, isn't it? 🤣

Copy link
Contributor Author

@grom72 grom72 left a comment

Choose a reason for hiding this comment

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

Reviewable status: 7 of 41 files reviewed, 2 unresolved discussions (waiting on @janekmi)


.github/workflows/pmem_check.yml line 20 at r8 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Wow. Another variation? Did it work at all?

Yes,


.github/workflows/pmem_valgrind.yml line 21 at r8 (raw file):

Previously, janekmi (Jan Michalski) wrote…

How has it worked previously? AFAIK there is no "release" PMDK release, isn't it? 🤣

Please, see RELEASE_LIBDIR -> NONDEBUG_LIBDIR in consts.py
and combine it with class Release(Build) -> class NonDebug(Build) in builds.py
release term has only been used by RUNTESTS.py and converted to use nondebug libraries:
RELEASE_LIBDIR = abspath(join(ROOTDIR, '..', 'nondebug'))

Copy link
Contributor Author

@grom72 grom72 left a comment

Choose a reason for hiding this comment

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

Reviewed 7 of 12 files at r5, 26 of 33 files at r10, 3 of 7 files at r11, 5 of 5 files at r12, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @janekmi)

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 7 files at r11, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @grom72)

@grom72 grom72 requested a review from osalyk June 29, 2023 07:51
Copy link
Contributor

@osalyk osalyk left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 7 of 12 files at r5, 26 of 33 files at r10, 3 of 7 files at r11, 5 of 5 files at r12, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @grom72)

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed 26 of 33 files at r10, 1 of 7 files at r11, 5 of 5 files at r12, 3 of 33 files at r13, 22 of 29 files at r14, 3 of 7 files at r15, 5 of 5 files at r16, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @grom72)

@janekmi janekmi merged commit cc78169 into pmem:master Jun 30, 2023
Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>
unify builds names - _debug instead of -debug
_nondebug instead of -nondebug

Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>
Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>
Signed-off-by: Tomasz Gromadzki <tomasz.gromadzki@intel.com>
@grom72 grom72 deleted the unified-pmem-tests branch July 4, 2023 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: 3 medium QA: CI .github/ and utils/ related to automated testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants