Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mkcal] Fix tst_load unit test failure. JB#57856 #26

Merged
merged 1 commit into from Mar 30, 2022
Merged

Conversation

pvuorela
Copy link
Contributor

QCOMPARE() was failing on comparing invalid QDateTimes between getLoadDates()
returned and separately created. Avoid timezones on such not to confuse
any comparisons.

@Tomin1 @dcaliste

QCOMPARE() was failing on comparing invalid QDateTimes between getLoadDates()
returned and separately created. Avoid timezones on such not to confuse
any comparisons.
@dcaliste
Copy link
Contributor

I cannot make it fails locally… I tried with TZ=foo ./tst_load but it's always succeeding.

That being said, not setting the time zone on invalid QDateTime may be a good idea.

@pvuorela
Copy link
Contributor Author

Perhaps TZ=foo gets effectively using UTC.

@pvuorela
Copy link
Contributor Author

The failure was interesting:

FAIL!  : tst_load::testRange(open bounded) Compared values are not the same
   Actual   (lEnd)   : Invalid QDateTime
   Expected (loadEnd): Invalid QDateTime
   Loc: [/home/abuild/rpmbuild/BUILD/mkcal-qt5-0.6.5/tests/tst_load.cpp(374)]
FAIL!  : tst_load::testRange(open bounded with overlap) Compared values are not the same
   Actual   (lEnd)   : Invalid QDateTime
   Expected (loadEnd): Invalid QDateTime
   Loc: [/home/abuild/rpmbuild/BUILD/mkcal-qt5-0.6.5/tests/tst_load.cpp(374)]

@pvuorela
Copy link
Contributor Author

Tried running test set with current and this. Random TZ value seemed to cause a bunch of failures, but that's broken setup anyhow. Europe/Paris and Europe/Helsinki both succeeded with both versions. But Europe/London was failing with the earlier, fixed by this.

@dcaliste
Copy link
Contributor

Thanks for mentioning Europe/London issue. Should we change the TZ variable for London in the test with a qputenv(), like in tst_storage::tst_recurrenceExpansion for instance ? Just to be sure that later on, any change in the range functions will not raise this again.

@pvuorela
Copy link
Contributor Author

Our automated unit test execution seems to catch this case already so it's enough for me.

Was that earlier "LGTM" btw? :)

@dcaliste
Copy link
Contributor

Yes, besides, that's ok to go in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants