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

Z: Update vsnprintf test to match the omr atoe_util changes #18362

Merged
merged 1 commit into from
Nov 9, 2023
Merged

Z: Update vsnprintf test to match the omr atoe_util changes #18362

merged 1 commit into from
Nov 9, 2023

Conversation

ehsankianifar
Copy link
Contributor

@ehsankianifar ehsankianifar commented Oct 27, 2023 •
edited
Loading

The implementation of atoe_vsnprintf has been updated in omr PR:#7158 This test needs to get updated to match those changes.

Signed-off-by: ehsan kiani far [email protected]

@ehsankianifar
Copy link
Contributor Author

This change is required for eclipse-omr/omr#7158 otherwise some tests would fail.

@ehsankianifar ehsankianifar force-pushed the Z-UpdateStringTestToAccomodateOmrVsnprintfChanges branch from 46d4a8f to 96690d0 Compare October 27, 2023 16:12
@ehsankianifar ehsankianifar mentioned this pull request Oct 27, 2023
Merged
@ehsankianifar ehsankianifar force-pushed the Z-UpdateStringTestToAccomodateOmrVsnprintfChanges branch 2 times, most recently from da94a2a to 7d6a879 Compare October 27, 2023 17:15
@ehsankianifar ehsankianifar force-pushed the Z-UpdateStringTestToAccomodateOmrVsnprintfChanges branch 2 times, most recently from 5d794a0 to 963cfbb Compare November 6, 2023 18:15
keithc-ca
keithc-ca requested changes Nov 7, 2023
View reviewed changes
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
keithc-ca
keithc-ca requested changes Nov 7, 2023
View reviewed changes
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
@keithc-ca
Copy link
Contributor

Please also correct the signed-off-by line in your commit message (it should use angle brackets around the email address):

Signed-off-by: ehsan kiani far <[email protected]>

@keithc-ca
Copy link
Contributor

Please correct the signed-off-by line in your commit messages (they should use angle brackets around the email address):

Signed-off-by: ehsan kiani far <[email protected]>

@ehsankianifar
Copy link
Contributor Author

Thanks @keithc-ca. I'll fix the commit message when squashing the commits. Let me know when it is okay with you.

keithc-ca
keithc-ca requested changes Nov 7, 2023
View reviewed changes
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
@ehsankianifar ehsankianifar force-pushed the Z-UpdateStringTestToAccomodateOmrVsnprintfChanges branch from db08e3b to 83db7b5 Compare November 8, 2023 17:45
keithc-ca
keithc-ca requested changes Nov 8, 2023
View reviewed changes
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
runtime/tests/port/j9strTest.c Outdated Show resolved Hide resolved
runtime/tests/port/j9strTest.c Show resolved Hide resolved
@ehsankianifar ehsankianifar force-pushed the Z-UpdateStringTestToAccomodateOmrVsnprintfChanges branch 3 times, most recently from b9705c4 to 8448074 Compare November 8, 2023 19:43
@keithc-ca
Copy link
Contributor

This change is required for eclipse/omr#7158 otherwise some tests would fail.

I think the dependency is the other way around: this cannot be merged before eclipse-omr/omr#7158.

@ehsankianifar
Z: Update vsnprintf test to match the omr atoe_util changes
3eb015e 
The implementation of atoe_vsnprintf has been updated in omr PR:eclipse-openj9#7158
This test needs to get updated to match those changes.

Signed-off-by: ehsan kiani far <[email protected]>
@ehsankianifar ehsankianifar force-pushed the Z-UpdateStringTestToAccomodateOmrVsnprintfChanges branch from 8448074 to 3eb015e Compare November 8, 2023 20:31
@ehsankianifar
Copy link
Contributor Author

This change is required for eclipse/omr#7158 otherwise some tests would fail.

I think the dependency is the other way around: this cannot be merged before eclipse/omr#7158.

That is true, we gonna need those changes to omr to make sure this tests pass

keithc-ca
keithc-ca approved these changes Nov 8, 2023
View reviewed changes
Copy link
Contributor

@keithc-ca keithc-ca left a comment

Choose a reason for hiding this comment

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

Merging this needs to be coordinated with merging eclipse-omr/omr#7158, else z/OS builds will be broken.

r30shah
r30shah approved these changes Nov 8, 2023
View reviewed changes
Copy link
Contributor

@r30shah r30shah left a comment

Choose a reason for hiding this comment

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

Thanks @keithc-ca and @ehsankianifar .

@r30shah
Copy link
Contributor

r30shah commented Nov 8, 2023

@joransiu / @keithc-ca As we need this change to be delivered for one of the zOS issue, can you launch test on this one ?

@keithc-ca keithc-ca merged commit 084fe2a into eclipse-openj9:master Nov 9, 2023
@keithc-ca
Copy link
Contributor

I will trigger the OMR acceptance job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@r30shah r30shah r30shah approved these changes

@keithc-ca keithc-ca keithc-ca approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ehsankianifar @keithc-ca @r30shah