Skip to content

Fix incorrect name/comment assignment in TestModelMetaService tests - #8296

Merged
justinmclean merged 1 commit into
apache:mainfrom
JeonDaehong:main
Aug 27, 2025
Merged

justinmclean merged 1 commit into
apache:mainfrom
JeonDaehong:main

Conversation

@JeonDaehong

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?
In TestModelMetaService.java, the tests testInsertAndUpdateModelComment and testInsertAndUpdateModelProperties were incorrectly setting the name and comment fields. This PR fixes the assignments so that name and comment are updated correctly according to the test's intent.

Why are the changes needed?
Previously, the tests would either overwrite the name with a comment value or keep the comment unchanged, causing the tests to not properly verify updates. Fixing the assignments ensures that comment updates and property updates are correctly tested.

Fixes: #8282

Does this PR introduce any user-facing change?
No. This only fixes test behavior.

How was this patch tested?

  • Corrected field assignments in the affected tests.
  • Verified that testInsertAndUpdateModelComment correctly updates the comment without changing the name.
  • Verified that testInsertAndUpdateModelProperties correctly maintains the name and updates properties.
  • Ran all existing tests to confirm no regressions.

@JeonDaehong

Copy link
Copy Markdown
Contributor Author

@justinmclean

It says the test failed, but I can’t see the details of the failure.
Could I get some feedback on this?

@justinmclean

Copy link
Copy Markdown
Member

I think they just took too long to run and were cancelled

@JeonDaehong

Copy link
Copy Markdown
Contributor Author

I think they just took too long to run and were cancelled

@justinmclean
This is the first time I’ve encountered something like this—how should I handle it?

@justinmclean

Copy link
Copy Markdown
Member

I can re-run the failed ones for you

@JeonDaehong

Copy link
Copy Markdown
Contributor Author

I can re-run the failed ones for you

@justinmclean

Please try running it again.
If it still doesn’t work, then it might be an issue with the changes I made to the source code, in which case I’ll take a look and fix it.

@JeonDaehong

Copy link
Copy Markdown
Contributor Author

It looks like the test is completed :D

@justinmclean justinmclean left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for fixing

@justinmclean
justinmclean merged commit bce78de into apache:main Aug 27, 2025
@JeonDaehong

Copy link
Copy Markdown
Contributor Author

Thanks !

diqiu50 pushed a commit to diqiu50/gravitino that referenced this pull request Sep 15, 2025
…pache#8296)

**What changes were proposed in this pull request?**
In `TestModelMetaService.java`, the tests
testInsertAndUpdateModelComment and testInsertAndUpdateModelProperties
were incorrectly setting the name and comment fields. This PR fixes the
assignments so that name and comment are updated correctly according to
the test's intent.

**Why are the changes needed?**
Previously, the tests would either overwrite the name with a comment
value or keep the comment unchanged, causing the tests to not properly
verify updates. Fixing the assignments ensures that comment updates and
property updates are correctly tested.

Fixes: apache#8282

**Does this PR introduce any user-facing change?**
No. This only fixes test behavior.

**How was this patch tested?**
- Corrected field assignments in the affected tests.
- Verified that `testInsertAndUpdateModelComment` correctly updates the
comment without changing the name.
- Verified that `testInsertAndUpdateModelProperties` correctly maintains
the name and updates properties.
- Ran all existing tests to confirm no regressions.
bharos pushed a commit to bharos/gravitino that referenced this pull request Oct 3, 2025
…pache#8296)

**What changes were proposed in this pull request?**
In `TestModelMetaService.java`, the tests
testInsertAndUpdateModelComment and testInsertAndUpdateModelProperties
were incorrectly setting the name and comment fields. This PR fixes the
assignments so that name and comment are updated correctly according to
the test's intent.

**Why are the changes needed?**
Previously, the tests would either overwrite the name with a comment
value or keep the comment unchanged, causing the tests to not properly
verify updates. Fixing the assignments ensures that comment updates and
property updates are correctly tested.

Fixes: apache#8282

**Does this PR introduce any user-facing change?**
No. This only fixes test behavior.

**How was this patch tested?**
- Corrected field assignments in the affected tests.
- Verified that `testInsertAndUpdateModelComment` correctly updates the
comment without changing the name.
- Verified that `testInsertAndUpdateModelProperties` correctly maintains
the name and updates properties.
- Ran all existing tests to confirm no regressions.
bharos pushed a commit to bharos/gravitino that referenced this pull request Oct 7, 2025
…pache#8296)

**What changes were proposed in this pull request?**
In `TestModelMetaService.java`, the tests
testInsertAndUpdateModelComment and testInsertAndUpdateModelProperties
were incorrectly setting the name and comment fields. This PR fixes the
assignments so that name and comment are updated correctly according to
the test's intent.

**Why are the changes needed?**
Previously, the tests would either overwrite the name with a comment
value or keep the comment unchanged, causing the tests to not properly
verify updates. Fixing the assignments ensures that comment updates and
property updates are correctly tested.

Fixes: apache#8282

**Does this PR introduce any user-facing change?**
No. This only fixes test behavior.

**How was this patch tested?**
- Corrected field assignments in the affected tests.
- Verified that `testInsertAndUpdateModelComment` correctly updates the
comment without changing the name.
- Verified that `testInsertAndUpdateModelProperties` correctly maintains
the name and updates properties.
- Ran all existing tests to confirm no regressions.
danhuawang pushed a commit to danhuawang/gravitino that referenced this pull request Jun 9, 2026
…pache#8296)

**What changes were proposed in this pull request?**
In `TestModelMetaService.java`, the tests
testInsertAndUpdateModelComment and testInsertAndUpdateModelProperties
were incorrectly setting the name and comment fields. This PR fixes the
assignments so that name and comment are updated correctly according to
the test's intent.

**Why are the changes needed?**
Previously, the tests would either overwrite the name with a comment
value or keep the comment unchanged, causing the tests to not properly
verify updates. Fixing the assignments ensures that comment updates and
property updates are correctly tested.

Fixes: apache#8282

**Does this PR introduce any user-facing change?**
No. This only fixes test behavior.

**How was this patch tested?**
- Corrected field assignments in the affected tests.
- Verified that `testInsertAndUpdateModelComment` correctly updates the
comment without changing the name.
- Verified that `testInsertAndUpdateModelProperties` correctly maintains
the name and updates properties.
- Ran all existing tests to confirm no regressions.
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.

[Improvement] Fix tests in TestModelMetaService.java

2 participants