Fixing build and tests on 1.10.x and dev

[BenedekFarkas]

Steps:

• 1. Fixing build and tests on 1.10.x:
  PR 8686: Fixing build and tests on 1.10.x #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.
• 2. Merge 1.10.x into dev:
  Then merge 1.10.x into issue/8684 (issue Merge 1.10.x into dev #8684, PR 8684: Merge 1.10.x into dev #8685), validate that everything's working and then merge that PR (which shows the diff between the two branches). At this point dev is partially stabilized.
• 3. Fixing build and tests on dev:
  Merge dev into issue/8686-dev (PR 8686: Fixing build and tests on dev #8695), validate that everything's working and then merge that PR. At this point, dev is fully stabilized and this issue can be resolved.

[BenedekFarkas]

SpecFlow test execution is broken at the moment since this commit: 0c34ca3 (that fixes #8445 with PR #8446).
I managed fix some startup errors by adding assembly binding redirects (see aa71039), 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.IEnumerable1[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.IEnumerable1[Orchard.Owin.IOwinMiddlewareProvider], Orchard.Environment.Configuration.ShellSettings)'.

#5490 and #7785 are about the same error, but in a different situation. The suggested fix was related to file access permissions, which is not the problem here (I tested the compiled Spec test app from IIS). Any suggestions?

[MpDzik]

Looks like IWorkContextAccessor is inaccessible. Did you try injecting Lazy?

[BenedekFarkas]

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).

[BenedekFarkas]

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.

[BenedekFarkas]

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

[sebastienros]

No idea

[BenedekFarkas]

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

[lbcsy]

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 ==========

[BenedekFarkas]

@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).

[lbcsy]

I tried dev as I thought you were referring to dev in your step 3. Good to know dev will be fixed soon.

[lbcsy]

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, Action2 bodyWithState, Func4 bodyWithLocal, Func1 localInit, Action1 localFinally) Parallel.ForEachWorker[TSource,TLocal](IEnumerable1 source, ParallelOptions parallelOptions, Action1 body, Action2 bodyWithState, Action3 bodyWithStateAndIndex, Func4 bodyWithStateAndLocal, Func5 bodyWithEverything, Func1 localInit, Action1 localFinally) Parallel.ForEach[TSource](IEnumerable1 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_01.b__1()
Task.InnerInvokeWithArg(Task childTask)
<>c__DisplayClass176_0.b__0(Object )

Others are:

1. ParseDateTest01
2. ParseDateTimeTest01
3. ParseDateTimeTest02
4. ParseDateTimeTest03
5. ParseDateTimeTest04
6. ParseTimeTest01

[BenedekFarkas]

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.

[BenedekFarkas]

@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:

1. Builds (with Razor compilation and warnings treated as errors), unit tests and SpecFlow tests all pass on 1.10.x and dev, see this run: https://github.com/OrchardCMS/Orchard/actions/runs/8202299611
2. Compile workflow runs on PRs and commits on 1.10.x and dev: runs the build and unit tests.
3. SpecFlow tests workflow runs every day at midnight and also executes the SpecFlow test on top of what Compile does (so it's the same as the nightly builds in TeamCity).
4. Deleted the TeamCity project.

[sebastienros]

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