Moving to EventGrid extension version 1.0.0 + fix flaky test #3143
Conversation
| @@ -120,7 +120,7 @@ | |||
| <HintPath>..\..\packages\Microsoft.Azure.WebJobs.Extensions.DocumentDB.1.3.0-beta1-10632\lib\net45\Microsoft.Azure.WebJobs.Extensions.DocumentDB.dll</HintPath> | |||
| </Reference> | |||
| <Reference Include="Microsoft.Azure.WebJobs.Extensions.EventGrid, Version=1.0.0.0, Culture=neutral, processorArchitecture=MSIL"> | |||
| <HintPath>..\..\packages\Microsoft.Azure.WebJobs.Extensions.EventGrid.1.0.0-10036\lib\net46\Microsoft.Azure.WebJobs.Extensions.EventGrid.dll</HintPath> | |||
There was a problem hiding this comment.
Nuget flags 1.0.0-10036 as an invalid package version (nuget pack issue), causing all VS builds to fail on all of my dev machines (failing to compile the Packages project). We don't have an actual product issue since apparently this doesn't repro on AppVeyor.
Confirmed with @watashiSHUN that 1.0.0 is the same bits as this version so this is safe to do. Going forward, he's going to fix this versioning scheme.
mathewc
force-pushed
the
test-fix
branch
2 times, most recently
from
15dea7a to
f613e86
Compare
| @@ -0,0 +1,6 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
There was a problem hiding this comment.
Added this missing packages file while trying to fix the build errors. You get warnings/errors without it.
| @@ -1550,10 +1550,6 @@ internal static void ApplyConfiguration(JObject config, ScriptHostConfiguration | |||
| scriptConfig.Functions.Add((string)function); | |||
| } | |||
| } | |||
| else | |||
| { | |||
| scriptConfig.Functions = null; | |||
There was a problem hiding this comment.
The test was flaking out because while it was specifying a functions whitelist, the whitelist wasn't actually being applied. There were then 4 functions running, and from time to time the queue function might get a message and timeout as well, leading to the flakiness. The test was verifying only a single timeout exception and there were multiple occurring.
Not sure why this else branch was here anyways - note that in other cases below, we only apply/override imperative settings if the json has a value.
There was a problem hiding this comment.
ApplyConfiguration_ClearsFunctionsFilter test failed in the PR build. Please take a look.
| @@ -149,6 +144,7 @@ private MockExceptionHandler GetExceptionHandler(ScriptHostManager manager) | |||
|
|
|||
| private async Task<MockScriptHostManager> CreateAndStartScriptHostManager(string scriptLang, string functionName, TimeSpan timeout, TraceWriter traceWriter) | |||
| { | |||
| // filter down to a single function | |||
There was a problem hiding this comment.
Before my fix, I verified in the debugger that this whitelist was being ignored
| }); | ||
|
|
||
| // verify that our mock handler received the timeout exception |
There was a problem hiding this comment.
This .Single() is what was throwing, because there were multiple timeouts in rare cases
| @@ -1550,10 +1550,6 @@ internal static void ApplyConfiguration(JObject config, ScriptHostConfiguration | |||
| scriptConfig.Functions.Add((string)function); | |||
| } | |||
| } | |||
| else | |||
| { | |||
| scriptConfig.Functions = null; | |||
There was a problem hiding this comment.
ApplyConfiguration_ClearsFunctionsFilter test failed in the PR build. Please take a look.
There was a problem hiding this comment.
Thanks, @mathewc !
Changes look good. @pragnagopa pointed out that you had a failing test, but looks like you've sent an update so. This LGTM, so let's get it in once you're green.
Fix for https://ci.appveyor.com/project/appsvc/azure-webjobs-sdk-script-y8o14/build/20180712-0606/tests