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

Potential issue with unit tests and cloned PnPContext #591

Closed
1 task done
s-KaiNet opened this issue Oct 21, 2021 · 4 comments
Closed
1 task done

Potential issue with unit tests and cloned PnPContext #591

s-KaiNet opened this issue Oct 21, 2021 · 4 comments

Comments

@s-KaiNet
Copy link
Collaborator

s-KaiNet commented Oct 21, 2021

Category

  • Bug

Describe the bug

I found an inconsistency when running tests in Online and Offline mode.
Below test passes in Online mode and fails in Offline.

[TestMethod]
public async Task PnPMockBugTest()
{
	//TestCommon.Instance.Mocking = false;
	using (var context = await TestCommon.Instance.GetContextAsync(TestCommon.TestSite))
	{
		var ctx2 = await context.CloneAsync(TestCommon.TestSite);
		await ctx2.Web.EnsurePropertiesAsync(w => w.Title);

		var ctx3 = await context.CloneAsync(TestCommon.NoGroupTestSite);
		await ctx3.Web.EnsurePropertiesAsync(w => w.Title);

		Assert.IsTrue(!ctx2.Web.Title.Equals(ctx3.Web.Title));
	}
}

I was able to narrow down the problem to the way how recorded .response files are created. The pattern is [method]-[test id]-[step]. The problem happens here:

clonedContext = await CloneForTestingAsync(this, uri, TestName, TestId + 100).ConfigureAwait(false);

The very first context will have the TestId = 0, next cloned context ctx2 will be TestId = 100, however, the third one also will be TestId = 100, because it's based on the same original context with Id = 0. As a result, the latter context will overwrite all .response files created by the ctx2. Simply because it has the same TestId and generates the same .response file path.

Steps to reproduce

Just run the above test method in Online and then Offline mode.

Expected behavior

I expected that in the case above the test should pass for both modes. However, I'm not sure how to properly fix it, because it looks like the problem is inside the tests infrastructure core.

@jansenbe
Copy link
Contributor

@s-KaiNet : the cloning was not well handled when it comes to offline test file generation, things could go wrong especially when using multiple clones or when using cloning inside tested methods. The reason for this is that the logic applied to come up with a unique id for the response file was flawed and sometimes generated the same response file name, so the last one overwrote the first one. When testing online all works, but when offline the wrong response file was picked up resulting in errors.

I've just pushed a fix for this: c02e124.

I've also added your test case to our set of tests: https://github.com/pnp/pnpcore/blob/dev/src/sdk/PnP.Core.Test/Base/PnPContextTests.cs#L695-L715

Thanks for reporting this, this will make it easier to write test cases.

@s-KaiNet
Copy link
Collaborator Author

Thanks! I hit that issue when working with AppManager, because for some operations I have to switch context to the tenant app catalog using Clone. As a result under certain conditions, some of the tests didn't work in Offline mode.

@jansenbe
Copy link
Contributor

@s-KaiNet : if the updates worked for you then feel free to close this issue, if not then let's try to get things fixed.

@s-KaiNet
Copy link
Collaborator Author

Yes, it works perfectly, thank you!

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

No branches or pull requests

2 participants