Closed BenedekFarkas closed 6 months ago
SpecFlow test execution is broken at the moment since this commit: 0c34ca3dd51fbdd11122446daaf79aa89286ec1e (that fixes #8445 with PR #8446). I managed fix some startup errors by adding assembly binding redirects (see aa710394167cd4813f58f776f486ece2af9563c9), but now it throws the following error:
None of the constructors found with 'Autofac.Core.Activators.Reflection.DefaultConstructorFinder' on type 'Orchard.Environment.DefaultOrchardShell' can be invoked with the available services and parameters:
Cannot resolve parameter 'Orchard.IWorkContextAccessor workContextAccessor' of constructor 'Void .ctor(Orchard.IWorkContextAccessor, System.Collections.Generic.IEnumerable1[Orchard.Mvc.Routes.IRouteProvider], System.Collections.Generic.IEnumerable
1[Orchard.WebApi.Routes.IHttpRouteProvider], Orchard.Mvc.Routes.IRoutePublisher, System.Collections.Generic.IEnumerable1[Orchard.Mvc.ModelBinders.IModelBinderProvider], Orchard.Mvc.ModelBinders.IModelBinderPublisher, Orchard.Tasks.ISweepGenerator, System.Collections.Generic.IEnumerable
1[Orchard.Owin.IOwinMiddlewareProvider], Orchard.Environment.Configuration.ShellSettings)'.
Looks like IWorkContextAccessor
is inaccessible. Did you try injecting Lazy
Yeah, I got as far, and my guess would be that the HttpContext is the underlying problem. I tried wrapping it in Lazy, but that doesn't help (because work context is used during shell activation, so very early on).
Working on this and #8683 in parallel to validate results in GHA - so far unit tests are all fixed where necessary and each one is passing. Continuing with the SpecFlow test execution issue mentioned above.
I've made good progress over the past few days and the build (including static Razor compilation) is completely warning-free + managed to fix all the failing tests (due to breaking changes in the code in recent years) except for one: Orchard.Specs.DateTimeFieldFeature.CreatingAndUsingDateTimeFieldsInAnotherCulture
.
The problem arises when adding a new culture and selecting it as the site culture through the DefineDefaultCulture
binding, because cache invalidation through the signal in DefaultCultureManager
doesn't seem to take effect. Signal usage was added to DefaultCultureManager
in 2010 (!) and then caching was added in 2014, so I'm pretty sure the tests were all green back then (the test itself was created in its current form in 2012). This is only true for running SpecFlow tests - running the "normal" Precompiled application or even the application running the SpecFlow tests from IIS and IIS Express both works fine. If I add an extra step to the SpecFlow test to enable a feature (which restarts the shell and resets the cache) the problem is gone.
Any ideas why signals/caching could be different when running the application through NUnit? CC @sebastienros
No idea
I've updated the issue description with the action plan to get 1.10.x and dev back into shape and the first step is to review and merge #8687. @sebastienros I'd like to ask you to review this PR carefully + I'd appreciate input from the Laser team (tagging @HermesSbicego-Laser and @MatteoPiovanelli-Laser, but please pass it on in the team) due to working on improving Orchard 1 so much recently + everyone else, of course. I've added a dozen or so comments to my PR with some additional context.
BTW all the unit and SpecFlow tests are passing! https://github.com/OrchardCMS/Orchard/actions/runs/5092692925/jobs/9154372082
If running Orchard.Framework.Tests in VS, it still gets 27 Failed
... Mocking Orchard.Tasks.IBackgroundService NUnit VS Adapter 2.3.0.0 executing tests is finished ========== Test run finished: 791 Tests (759 Passed, 27 Failed, 5 Skipped) run in 7.2 min ==========
@lbcsy on which branch? 1.10.x
should be all green now, dev
isn't yet (but issue/8686-dev
should be all green, which will be merged into dev
).
I tried dev as I thought you were referring to dev in your step 3. Good to know dev will be fixed soon.
Today I tried 1.10x, it has 7 failed tests here, they are all in DateFormatterTests, for example:
FormatDateTimeTest03 Source: DefaultDateFormatterTests.cs line 502 Duration: 194 ms
Message: System.AggregateException : One or more errors occurred. ----> System.ArgumentException : The UTC Offset of the local dateTime parameter does not match the offset argument. Parameter name: offset
Stack Trace:
Task.ThrowIfExceptional(Boolean includeTaskCanceledExceptions)
Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
Parallel.ForWorker[TLocal](Int32 fromInclusive, Int32 toExclusive, ParallelOptions parallelOptions, Action1 body, Action
2 bodyWithState, Func4 bodyWithLocal, Func
1 localInit, Action1 localFinally) Parallel.ForEachWorker[TSource,TLocal](IEnumerable
1 source, ParallelOptions parallelOptions, Action1 body, Action
2 bodyWithState, Action3 bodyWithStateAndIndex, Func
4 bodyWithStateAndLocal, Func5 bodyWithEverything, Func
1 localInit, Action1 localFinally) Parallel.ForEach[TSource](IEnumerable
1 source, ParallelOptions parallelOptions, Action1 body) DefaultDateFormatterTests.FormatDateTimeTest03() line 513 --ArgumentException DateTimeOffset.ctor(DateTime dateTime, TimeSpan offset) <>c__DisplayClass12_0.<FormatDateTimeTest03>b__0(CultureInfo culture) line 539 <>c__DisplayClass17_0
1.
Others are:
Yeah, I've got the same locally - I couldn't find a fix so far, but it's almost certainly down to culture / date formatting settings, so I just deferred to the GitHub Actions execution, which is successful.
@sebastienros after our discussion yesterday, I merged in #8685, then I made some further updates to #8686 and squash-merged that, so to sum up this journey:
Yeah, I've got the same locally - I couldn't find a fix so far, but it's almost certainly down to culture / date formatting settings, so I just deferred to the GitHub Actions execution, which is successful.
When DateTime.Parse is used, specify Culture.Invariant
if you assume it's the ISO format
Steps:
1.10.x
: PR #8687 (issue/8686
branch) is to be merged first. When that happens,1.10.x
is fully stabilized, but this issue shouldn't be resolved yet.1.10.x
intodev
: Then merge1.10.x
intoissue/8684
(issue #8684, PR #8685), validate that everything's working and then merge that PR (which shows the diff between the two branches). At this pointdev
is partially stabilized.dev
: Mergedev
intoissue/8686-dev
(PR #8695), validate that everything's working and then merge that PR. At this point,dev
is fully stabilized and this issue can be resolved.