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

Fix for test failure related to SnapshotInfo object creation #1237

Closed

Conversation

aggarwalShivani
Copy link
Contributor

Description

To resolve the compile-time error caused due to change in SnapshotInfo class, I've updated the TestUtils.kt to create the object of SnapshotInfo with additional pinnedTimestamp (0) parameter.

Kindly let me know if this solution is fine :)

Related Issues

Resolves #1236

Check List

  • [ ] New functionality includes testing.
  • [ ] New functionality has been documented.
  • [ ] API changes companion pull request created.
  • Commits are signed per the DCO using --signoff.
  • [ ] Public documentation issue/PR created.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: aggarwalShivani <shivani.aggarwal@nokia.com>
@aggarwalShivani
Copy link
Contributor Author

Hi Reviewers,
Requesting feedback for this minor fix :)

@dblock
Copy link
Member

dblock commented Sep 16, 2024

cc: @vikasvb90

Tests are failing in this PR though, so the fix is not enough to merge.

@bowenlan-amzn
Copy link
Member

@aggarwalShivani please merge with main once, see if that will help test run

@vikasvb90
Copy link
Collaborator

Thanks @dblock for notifying!
@aggarwalShivani There is already a method in SnapshotInfo ( code pointer) which doesn't need pinnedTimestamp.
Can you please add details on which workflow/test failure is it fixing?

@vikasvb90
Copy link
Collaborator

The failing tests in your PR are because of recent deprecation in upload-artifacts. Can you check if you are still getting a compile-time error? It could have been already fixed by a PR.

@vikasvb90
Copy link
Collaborator

vikasvb90 commented Sep 17, 2024

I have recently merged a PR by @Hailong-am. Build was green in this PR (except known backward incompatibility failure) Can you rebase your changes and see if these tests are failing?

@vikasvb90
Copy link
Collaborator

Closing this PR since the change isn't fixing anything. Please reopen if you see any other issue. Thanks!

@vikasvb90 vikasvb90 closed this Sep 17, 2024
@aggarwalShivani
Copy link
Contributor Author

Hi,
Yes, looks like, a few other changes were made to the SnapshotInfo class after this PR was raised :)
This change is no longer needed, and the issue seems resolved.
Thanks!

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.

[BUG] Gradle tests failing due to compile-time errors introduced in TestUtils.kt for SnapshotInfo class
4 participants