Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Flaky test: RendersScriptTagsForGlobbedSrcResults #8157

Closed
ryanbrandenburg opened this issue Jul 27, 2018 · 11 comments
Closed

Flaky test: RendersScriptTagsForGlobbedSrcResults #8157

ryanbrandenburg opened this issue Jul 27, 2018 · 11 comments
Assignees
Milestone

Comments

@ryanbrandenburg
Copy link
Contributor

This test fails occasionally with the following error:

System.NullReferenceException : Object reference not set to an instance of an object.
   at System.Collections.Generic.Dictionary`2.FindEntry(TKey key)
   at System.Collections.Generic.Dictionary`2.TryGetValue(TKey key, TValue& value)
   at Castle.DynamicProxy.Generators.BaseProxyGenerator.ObtainProxyType(CacheKey cacheKey, Func`3 factory)
   at Castle.DynamicProxy.ProxyGenerator.CreateClassProxy(Type classToProxy, Type[] additionalInterfacesToProxy, ProxyGenerationOptions options, Object[] constructorArguments, IInterceptor[] interceptors)
   at Moq.Proxy.CastleProxyFactory.CreateProxy(Type mockType, ICallInterceptor interceptor, Type[] interfaces, Object[] arguments)
   at Moq.Mock`1.<InitializeInstance>b__20_0()
   at Moq.Mock`1.OnGetObject()
   at Moq.Mock`1.get_Object()
   at Microsoft.AspNetCore.Mvc.TagHelpers.ScriptTagHelperTest.MakeHostingEnvironment() in /_/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ScriptTagHelperTest.cs:line 987
   at Microsoft.AspNetCore.Mvc.TagHelpers.ScriptTagHelperTest.RendersScriptTagsForGlobbedSrcResults() in /_/test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ScriptTagHelperTest.cs:line 617

Other tests within that build may have failed with a similar message, but they are not listed here. Check the link above for more info.

CC @Eilon,@muratg,@mkArtakMSFT

This issue was made automatically. If there is a problem contact @ryanbrandenburg.

@ryanbrandenburg
Copy link
Contributor Author

ryanbrandenburg commented Jul 27, 2018

Please use this workflow to address this flaky test issue, including checking applicable checkboxes and filling in the applicable "TODO" entries:

  • Is this actually a flaky test? Kind-of though issue appears to be in our dependencies

    • No, this is a regular test failure, fix the test/product (TODO: Link to commit/PR)
    • Yes, proceed below...
  • Is this test failure caused by product code flakiness? (Either this product, or another product this test depends on.)

  • Is it that the test itself is flaky? This includes external transient problems (e.g. remote server problems, file system race condition, etc.)

    • Is there is a way to change our test to avoid this flakiness?
      • Yes? Change the test!
        • Change the test to avoid the flakiness, for example by using a different test strategy, or by adding retries w/ timeouts (TODO: Link to PR/commit)
        • Run the test 100 times locally as a sanity check.
        • Close this bug
      • No? The lower-level failure could affect any test in any run. No point in removing this one.
        • Delete the test because flaky tests are not useful (TODO: Link to PR/commit)

This comment was made automatically. If there is a problem contact @ryanbrandenburg.

@mkArtakMSFT
Copy link
Member

Thanks @ryanbrandenburg.
@dougbu, can you please take care of this? Thanks!

@mkArtakMSFT mkArtakMSFT added 1 - Ready task PRI: 1 - Required Must be handled in a reasonable time labels Jul 27, 2018
@mkArtakMSFT mkArtakMSFT added this to the 2.2.0-preview1 milestone Jul 27, 2018
@dougbu
Copy link
Member

dougbu commented Jul 27, 2018

According to the test history, this test has failed once in 6479 runs. TeamCity calls that success rate "100%". Are we investigating tests that are that reliable?

@pranavkm
Copy link
Contributor

pranavkm commented Jul 27, 2018

More importantly, the stack trace suggests an issue with CastleCore. See issue: castleproject/Core#193. Relevant comment thread: castleproject/Core#193 (comment)

@Eilon
Copy link
Member

Eilon commented Jul 27, 2018

@dougbu yes, because this can cause "death by a thousand papercuts." If we have hundreds of tests that fail at that rate, that's not good enough.

@dougbu
Copy link
Member

dougbu commented Jul 27, 2018

🆗 @Eilon

Agree with @pranavkm that this failure appears to be completely w/in Castle.Core. The test failed when executing new Mock<IFileProvider>(), something MVC does in 10 other tests or test helpers. The MakeHostingEnvironment() method in this particular test class where that instantiation occurs is called from 19 tests in this class alone. None of the other tests failed in the single test run of interest.

Suggest we leave this open to track castleproject/Core#193. We're fairly far behind on Moq versions (at 4.7.49 with 4.9.0 the latest and 4.7.145 the latest of our minor version) and could upgrade to whichever Moq version picks up the castleproject/Core#193 fix. In the meantime, I'll test to see if Moq 4.9.0 brings along anything that breaks builds in the EF, KestrelHttpServer, Mvc, or Razor repos (hopefully that's a representative sample for this trial).

@dougbu
Copy link
Member

dougbu commented Jul 29, 2018

Saw a bit of unreliability in aspnet/KestrelHttpServer but nothing that didn't also reproduce on my machine without the Moq version bump. @halter73 also said most of the failures were familiar. So, no new failures there.

No failures at all in aspnet/EntityFrameworkCore, aspnet/Mvc or aspnet/Razor in their release/2.2 branches with the latest Moq (and therefore almost the latest Castle.Core).

@dougbu
Copy link
Member

dougbu commented Jul 29, 2018

Workarounds are fine for now. But, will leave this in Ready / blocked state until castleproject/Core#193 is resolved and moq/moq4 picks up the fix.

@aspnet-hello
Copy link

Configure_AddsPageViewLocationFormats_WithDefaultPagesRoot failed with about the same error on 2.2.

This comment was made automatically. If there is a problem contact ryanbrandenburg.

@aspnet-hello
Copy link

There were 1 failures with about the same error on 2.2:

  • PopulateFeature_ReturnsEmptyList_IfNoAssemblyPartsAreRegistered

This comment was made automatically. If there is a problem contact ryanbrandenburg.

@dougbu
Copy link
Member

dougbu commented Oct 25, 2018

Closing as the upgrade (dotnet/aspnetcore@904c9d3623) more than two weeks ago seems to have cleared all similar failures.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants