Skip to content

Added error handling and empty array check #255

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

Merged
merged 23 commits into from
Dec 12, 2022
Merged

Added error handling and empty array check #255

merged 23 commits into from
Dec 12, 2022

Conversation

zulander1
Copy link
Contributor

This would improve issue for #253

@zulander1 zulander1 changed the title Added error handling Added error handling and empty array check Nov 20, 2022
@zulander1
Copy link
Contributor Author

This contains fix for issue #256

Copy link
Member

@slorello89 slorello89 left a comment

Choose a reason for hiding this comment

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

Hey @zulander1 thanks for opening this, couple things:

  1. Use Enviornment.Newline rather than carriage returns
  2. Add a test in SearchTest.cs - something like this should suffice:
[Fact]
        public void TestNullResponseDoc()
        {
            int? nullVal = null;
            var nullResult = RedisResult.Create(nullVal);
            var res = new RedisReply[] { 1, $"foo:{Ulid.NewUlid()}", new (nullResult) };
            
            var query = new RedisQuery("fake-idx");
            _mock.Setup(x => x.Execute(It.IsAny<string>(), It.IsAny<string[]>()))
                .Returns(res);

            var result = _mock.Object.Search<Person>(query);
        }

@slorello89 slorello89 linked an issue Dec 9, 2022 that may be closed by this pull request
@zulander1
Copy link
Contributor Author

Hey @zulander1 thanks for opening this, couple things:

  1. Use Enviornment.Newline rather than carriage returns
  2. Add a test in SearchTest.cs - something like this should suffice:
[Fact]
        public void TestNullResponseDoc()
        {
            int? nullVal = null;
            var nullResult = RedisResult.Create(nullVal);
            var res = new RedisReply[] { 1, $"foo:{Ulid.NewUlid()}", new (nullResult) };
            
            var query = new RedisQuery("fake-idx");
            _mock.Setup(x => x.Execute(It.IsAny<string>(), It.IsAny<string[]>()))
                .Returns(res);

            var result = _mock.Object.Search<Person>(query);
        }

Done, applied the correctoins

@zulander1
Copy link
Contributor Author

@slorello89 , I am not sure why the building is failing on test Redis.OM.Unit.Tests.RediSearchTests.SearchFunctionalTests.TestSave, working on my local, can you help ?

Copy link
Member

@slorello89 slorello89 left a comment

Choose a reason for hiding this comment

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

LGTM 👍 - issue with the tests were related to another PR and how that interacted with the search tests.

@slorello89 slorello89 merged commit becd81e into redis:main Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

An error will occur when a key expire, while enumerating
2 participants