Unit test confusion

[wizofaus]

Hi,
I'd like to start working on a fix for handling Image and Location fields properly with LoadListDataAsStream(Async), but I'm having trouble finding a unit test that simply calls this on a given list and parses the result, and that's it. All the ones I've looked for seem to try creating data in the list too (e.g. ListTests.GetListDataByRenderListDataAsStream). Surely such a test exists? Also how do you configure "credman"?
I have an Entra ID app registration set up that has the necessary access to my live document library with image/location data in it, so I was assuming I'd be able to use that...

FWIW, the actual bugs are in PnP.Core.Model.SharePoint.ListDataAsStreamHandler.Deserialize() (/TransformRowData) - are there unit tests for more low level functions like that? (I tried searching the call hierarchy but no real luck).

Thanks

Dylan

[wizofaus]

BTW I got the credman bit working, though it wasn't 100% obvious - it seems to be using a username/password rather than client-id/tenant-id. At one point it seemed to have stopped working again and I was getting Microsoft.Identity.Client.MsalUiRequiredException thrown inside Microsoft.Identity.Client.dll, which had me down a rabbit hole for a long time until it twigged that the only thing I'd changed was the exception settings in Visual Studio - and sure enough, after ignoring that exception everything continued working fine. Not sure if it's an issue with the PnP library that this exception is happening but it doesn't seem to matter anyway.

In the end I wrote my own test, and sure enough, just the following throws an exception because of the presence of Image/Location data:

        public async Task ExistingListCsomTest()
        {
            TestCommon.Instance.Mocking = false;
            using (var context = await TestCommon.Instance.GetContextAsync(TestCommon.TestSite))
            {
                string listTitle = "WithImageAndLocation";
                var myList = await context.Web.Lists.GetByTitleAsync(listTitle);

                await myList.LoadListDataAsStreamAsync(new RenderListDataOptions { });
            }

Specifically ListDataAsStreamHandler.GetJsonPropertyValueAsString( ) is trying to read a decimal number (-37.8512) as an integer. I think I just gave away the latitude I live at :)

For the image object, adding this to GetJsonPropertyValue works:

                        else if (propertyValue.ValueKind == JsonValueKind.Object)
                        {
                            return propertyValue.ToObject<System.Dynamic.ExpandoObject>();
                        }

and is adequate for my needs, but maybe we should be defining a specific type with the expected fields based on the column type? (there's FieldLocationValue* but no FieldImageValue or FieldThumbnailValue). Actually ExpandoObject is not particularly nice - the individual properties are available of type JsonElement, vs string/double etc., which is not what I expected at all). In fact I tested with the MS CSOM library - and it's actually returned back as a string with the raw JSON in it! Which is very unexpected, I'd like to think we can do better (CSOM does the same for actual Location fields too - vs the "GeoLoc" sub-field that you get when you add a Location field, which is the field that is of type SP.FieldGeolocationValue - despite it being the value of FieldLocationValue.SharePointRestType, which seems incorrect to me).

Also can I change IFieldLocationValue Latitude/Longitude to decimal? double is supposed to be accurate up to 16 decimal places but I'm sure I saw at least one unexpected/lossy conversion even with only 4 decimal places.

[jansenbe]

@wizofaus : thanks for helping. See https://github.com/pnp/pnpcore/blob/dev/src/sdk/PnP.Core.Test/SharePoint/ListTests.cs#L45-L148 for a good sample test case. Also, see https://pnp.github.io/pnpcore/contributing/writing%20tests.html for help with writing tests

[wizofaus]

But that test creates a list and adds data which is unnecessarily complex (and what if there's a bug in that code - I just want a test parsing the response and nothing else). Surely we can have tests that work with existing data (especially if it's preloaded from a saved response file). Even if it requires, say, a PowerShell or similar script to create/populate the list on the live site. As it is the test I wrote that just loads data from an existing list seemed to be doing a lot more work than I'd expect (at least 3 response files got created).

[jansenbe]

@wizofaus : this is how this test concept was designed, there's indeed some work involved but the tests always work with "real" server responses.

[wizofaus]

From a developer perspective though I want to know that if I've changed, e.g. the parsing code for a particular API response, I can easily test just that, and not be tripped up by possible other issues creating sites/lists/columns/inserting values programmatically etc.
Surely we can have some tests that either simply get skipped if the expected "live" list doesn't exist (or doesn't have the right values etc.) - the important thing is that they work in mock/"saved-response" mode. Or like I said, I can provide powershell or other scripts to create those lists on live sites if needed.
As it is, what about tests that test processing massive lists with 10s of 1000s of items, surely the current tests don't start from scratch each time and add all the items?

[wizofaus]

Anyway the more important thing is agreeing on what should be returned for Location fields.