Azure / PSRule.Rules.Azure

Rules to validate Azure resources and infrastructure as code (IaC) using PSRule.
https://azure.github.io/PSRule.Rules.Azure/
MIT License
387 stars 84 forks source link

DatetimeAdd test from FunctionTests.cs fails with DST timezone #891

Closed ArmaanMcleod closed 3 years ago

ArmaanMcleod commented 3 years ago

Description of the issue

When running build.ps1 on my machine, I get the follow error from DatetimeAdd test:

[xUnit.net 00:00:07.34]     PSRule.Rules.Azure.FunctionTests.DateTimeAdd [FAIL]
  Failed PSRule.Rules.Azure.FunctionTests.DateTimeAdd [12 ms]
  Error Message:
   Assert.Equal() Failure
Expected: 2020-03-30T00:53:14.0000000+11:00
Actual:   2020-03-29T23:53:14.0000000Z
  Stack Trace:
     at PSRule.Rules.Azure.FunctionTests.DateTimeAdd() in C:\Users\armcleod\Documents\github-repos\PSRule.Rules.Azure\tests\PSRule.Rules.Azure.Tests\FunctionTests.cs:line 813
Results File: C:\Users\armcleod\Documents\github-repos\PSRule.Rules.Azure\reports\armcleod_DESKTOP-6LBF3IE_2021-08-18_16_57_30.trx

Failed!  - Failed:     1, Passed:   113, Skipped:     0, Total:   114, Duration: 265 ms - PSRule.Rules.Azure.Tests.dll (netcoreapp3.1)
ERROR: Command exited with code 1. {
            # Test library
            dotnet test --logger trx -r (Join-Path $PWD -ChildPath reports/) tests/PSRule.Rules.Azure.Tests
        }
At C:\Users\armcleod\Documents\github-repos\PSRule.Rules.Azure\pipeline.build.ps1:222 char:9
+         exec {
+         ~~~~~~
At C:\Users\armcleod\Documents\github-repos\PSRule.Rules.Azure\pipeline.build.ps1:214 char:1
+ task TestDotNet {
+ ~~~~~~~~~~~~~~~~~
At C:\Users\armcleod\Documents\github-repos\PSRule.Rules.Azure\pipeline.build.ps1:510 char:1
+ task Test Build, Rules, TestDotNet, TestModule
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Build FAILED. 15 tasks, 1 errors, 0 warnings 00:00:57.3449600
Invoke-BuildExec: C:\Users\armcleod\Documents\github-repos\PSRule.Rules.Azure\pipeline.build.ps1:222:9
Line |
 222 |          exec {
     |          ~~~~~~
     | Command exited with code 1. {             # Test library             dotnet test --logger trx -r (Join-Path $PWD
     | -ChildPath reports/) tests/PSRule.Rules.Azure.Tests         }

Code: https://github.com/Azure/PSRule.Rules.Azure/blob/main/tests/PSRule.Rules.Azure.Tests/FunctionTests.cs#L801-L820

Expected behaviour

Tests should pass. Although this seems like an issue on my machine with time zones. It seems to be the second assert that is problematic.

I can actually force this test to pass by doing this:

[Fact]
[Trait(TRAIT, TRAIT_DATE)]
public void DateTimeAdd()
{
    var context = GetContext();
    var utc = DateTime.Parse("2020-04-07 14:53:14Z", new CultureInfo("en-US")).ToUniversalTime(); # Force to UTC format. 

    var actual1 = DateTime.Parse(Functions.DateTimeAdd(context, new object[] { utc.ToString("u"), "P3Y" }) as string, new CultureInfo("en-US"));
    var actual2 = DateTime.Parse(Functions.DateTimeAdd(context, new object[] { utc.ToString("u"), "-P9D" }) as string, new CultureInfo("en-US"));
    var actual3 = DateTime.Parse(Functions.DateTimeAdd(context, new object[] { utc.ToString("u"), "PT1H" }) as string, new CultureInfo("en-US"));

    Assert.Equal(utc.AddYears(3), actual1.ToUniversalTime());
    Assert.Equal(utc.AddDays(-9), actual2.ToUniversalTime().AddHours(1)); # Add one hour
    Assert.Equal(utc.AddHours(1), actual3.ToUniversalTime());

    Assert.Throws<ExpressionArgumentException>(() => Functions.DateTimeAdd(context, null));
    Assert.Throws<ExpressionArgumentException>(() => Functions.DateTimeAdd(context, new object[] { }));
    Assert.Throws<ExpressionArgumentException>(() => Functions.DateTimeAdd(context, new object[] { 1, 2 }));
    Assert.Throws<ExpressionArgumentException>(() => Functions.DateTimeAdd(context, new object[] { utc.ToString("u"), 2 }));
}

Which makes me believe this could be a DST issue, since adding an hour to the second Assert passes the test. Could be completely wrong about this 😄 . This definitely isn't a fix because the CI would break since it doesn't seem to be affected by this issue.

I could probably try reinstalling dotnet to see if that fixes the issue. Or we could set a specific time zone and use that?

Environment

$PSVersionTable:

Name                           Value
----                           -----
PSVersion                      7.1.4
PSEdition                      Core
GitCommitId                    7.1.4
OS                             Microsoft Windows 10.0.19043
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

dotnet version: 5.0.303

ArmaanMcleod commented 3 years ago

I guess another thing to consider is that should we separate the tests from the build? Like create a test.ps1 script that runs just the tests. At least this way builds will be quicker when running locally and don't rely on tests passing.

BernieWhite commented 3 years ago

@ArmaanMcleod If just updating var utc = DateTime.Parse("2020-04-07 14:53:14Z", new CultureInfo("en-US")).ToUniversalTime(); alone fixes the issue I think that is fine. The intent it that it should be UTC anyway by specifying Z in the time string. But unusual it is not automatically picking it up as UTC.

Does it fix the issue? or is the +1 hour also required?

BernieWhite commented 3 years ago

@ArmaanMcleod

I guess another thing to consider is that should we separate the tests from the build? Like create a test.ps1 script that runs just the tests. At least this way builds will be quicker when running locally and don't rely on tests passing.

Did you try the VSCode build task? Should work a treat to just build instead of build + test. But I'm open to this if you think it would improve your workflow. Feel free to open an issue and PR to close the loop. 😄

ArmaanMcleod commented 3 years ago

@ArmaanMcleod If just updating var utc = DateTime.Parse("2020-04-07 14:53:14Z", new CultureInfo("en-US")).ToUniversalTime(); alone fixes the issue I think that is fine. The intent it that it should be UTC anyway by specifying Z in the time string. But unusual it is not automatically picking it up as UTC.

Does it fix the issue? or is the +1 hour also required?

@BernieWhite Unfortunately the +1 is required. var utc = DateTime.Parse("2020-04-07 14:53:14Z", new CultureInfo("en-US")).ToUniversalTime(); does force the time format to be the same e.g. 2020-03-29T13:53:14.0000000Z, but the hours are still 1 apart when doing Assert.Equal() since utc.AddDays(-9) gives back 2020-03-29T14:53:14.0000000Z.

I have been fine so far without this test passing. I usually just build then run Invoke-Pester on the tests I am changing to make sure the the changes I am making don't break anything.

Did you try the VSCode build task? Should work a treat to just build instead of build + test. But I'm open to this if you think it would improve your workflow. Feel free to open an issue and PR to close the loop. 😄.

Ah didn't realize that task existed, I think that should be fine 🎉 . I might add some extra documentation to https://github.com/Azure/PSRule.Rules.Azure/blob/main/docs/install-instructions.md#building-from-source to include this way for anyone else who is just wanting to do a simple build. Could include the handy Test task as well. Thanks mate 🙂 .

BernieWhite commented 3 years ago

@ArmaanMcleod Ok. I adjusted my time zone to match yours with DST enabled and can repo the issue.

ArmaanMcleod commented 3 years ago

Thanks @BernieWhite for looking into this 🙂. I knew it had something to do with DST but didn't get much time to look into the issue this week with all my customer work.