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

Remove check_version in common_functions.sh #4954

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

annaibm
Copy link
Contributor

@annaibm annaibm commented Jan 10, 2024

resolves : #4949

@annaibm
Copy link
Contributor Author

annaibm commented Jan 11, 2024

@annaibm annaibm marked this pull request as ready for review January 11, 2024 18:14
@llxia
Copy link
Contributor

llxia commented Jan 11, 2024

I think https://github.com/adoptium/aqa-tests/pull/4954/files#diff-e1cbec3d1700bf35e3daef146d4667564133f0a528b700594195c7a0184e7ff5L17 should also be removed. Please check its references.

external/build_image.sh Show resolved Hide resolved
external/common_functions.sh Outdated Show resolved Hide resolved
@llxia
Copy link
Contributor

llxia commented Jan 12, 2024

@annaibm did you test the latest change in Grinder? supported_versions seems to be referenced in other places: https://github.com/search?q=repo%3Aadoptium%2Faqa-tests%20supported_versions&type=code

@annaibm
Copy link
Contributor Author

annaibm commented Jan 12, 2024

@llxia I have tested in Grinders :
1.37316,
2.37315.
Other than the above, Is there any other specific tests for external that is crucial to be done?
Should I keep https://github.com/adoptium/aqa-tests/pull/4954/files#diff-e1cbec3d1700bf35e3daef146d4667564133f0a528b700594195c7a0184e7ff5L17 , if its used in other places. Not sure in Grinder it was not breaking/failing ?

@annaibm annaibm marked this pull request as draft January 12, 2024 14:36
@annaibm annaibm force-pushed the remove_checkversion_4949 branch 2 times, most recently from 5a29c49 to fb46146 Compare January 17, 2024 15:09
@annaibm annaibm force-pushed the remove_checkversion_4949 branch 3 times, most recently from bd9bd9e to 3e1618e Compare January 30, 2024 18:02
@llxia
Copy link
Contributor

llxia commented Jan 30, 2024

The suggestion was to remove supported_versions. However, supported_versions is still in the PR. Please ensure the Grinder is testing the correct change.

@annaibm annaibm force-pushed the remove_checkversion_4949 branch 3 times, most recently from 250c729 to 4c09020 Compare January 30, 2024 22:25
@annaibm
Copy link
Contributor Author

annaibm commented Jan 31, 2024

Grinder tests:
37729 - aot_test
37733 - criu-functional

@annaibm
Copy link
Contributor Author

annaibm commented Jan 31, 2024

Grinder test:
dev.external -> 37738

external/build_all.sh Outdated Show resolved Hide resolved
- remove check_version function
- remove set_version function
- remove supported_versions
resolves : adoptium#4949

Signed-off-by: Anna Babu Palathingal <[email protected]>
Copy link
Contributor

@LongyuZhang LongyuZhang left a comment

Choose a reason for hiding this comment

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

LGTM

@LongyuZhang
Copy link
Contributor

LongyuZhang commented Jan 31, 2024

Grinder test: dev.external -> 37738

The failure is not related to this PR, will fix in other PRs. Same for criu-functional.

@LongyuZhang LongyuZhang marked this pull request as ready for review February 6, 2024 14:31
@LongyuZhang LongyuZhang merged commit e32dc86 into adoptium:master Feb 6, 2024
3 checks passed
@annaibm annaibm deleted the remove_checkversion_4949 branch March 12, 2024 20:57
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.

Remove check_version() in common_functions.sh for external tests
4 participants