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

Restoring State After Static Changes in Tests #1571

Merged
merged 1 commit into from
Jul 15, 2020
Merged

Restoring State After Static Changes in Tests #1571

merged 1 commit into from
Jul 15, 2020

Conversation

oleibman
Copy link
Collaborator

@oleibman oleibman commented Jul 5, 2020

This request does not change any source code, only tests.

For a change on which I was working, a test passed when run on its own,
but failed when run as part of the full test suite. It turned out that
an existing test had changed a static value,
thousands separator in this case, and failed to restore it.
The test turned out to be AdvancedBinderTest.

The search for the offending test was more difficult than it should have
been because 26 test scripts which had nothing to do with thousands
separator nevertheless changed that value. They all changed
decimal separator, currency code, and compatibility mode as well,
again for no reason. I changed all of those to eliminate those operations.

I changed the following tests, which actually do change the static
properties identified above for a reason, to restore them as part of teardown.

  • CalculationTest sets compatibilityMode and locale
  • DayTest sets compatibilityMode, returnDateType, and excelCalendar
  • CountTest sets compatibilityMode
  • FunctionsTest sets compatibilityMode and returnDateType
  • AdvancedValueBinderTest sets currencyCode, decimalSeparator, thousandsSeparator
  • StringHelperTest sets currencyCode, decimalSeparator, thousandsSeparator
  • NumberFormatTest sets currencyCode, decimalSeparator, thousandsSeparator
  • HtmlNumberFormatTest sets currencyCode, decimalSeparator, thousandsSeparator

This is:

- [x] a bugfix
- [ ] a new feature

Checklist:

Why this change is needed?

Improving test suite (see above for full explanation).

This request does not change any source code, only tests.

For a change on which I was working, a test passed when run on its own,
but failed when run as part of the full test suite. It turned out that
an existing test had changed a static value,
thousands separator in this case, and failed to restore it.
The test turned out to be AdvancedBinderTest.

The search for the offending test was more difficult than it should have
been because 26 test scripts which had nothing to do with thousands
separator nevertheless changed that value. They all changed
decimal separator, currency code, and compatibility mode as well,
again for no reason. I changed all of those to eliminate those operations.

I changed the following tests, which actually do change the static
properties identified above for a reason, to restore them as part of teardown.
- CalculationTest sets compatibilityMode and locale
- DayTest sets compatibilityMode, returnDateType, and excelCalendar
- CountTest sets compatibilityMode
- FunctionsTest sets compatibilityMode and returnDateType
- AdvancedValueBinderTest sets currencyCode, decimalSeparator, thousandsSeparator
- StringHelperTest sets currencyCode, decimalSeparator, thousandsSeparator
- NumberFormatTest sets currencyCode, decimalSeparator, thousandsSeparator
- HtmlNumberFormatTest sets currencyCode, decimalSeparator, thousandsSeparator
@MarkBaker
Copy link
Member

Useful as we extend the test suite to cover more cases, thanks

@MarkBaker MarkBaker merged commit 165034a into PHPOffice:master Jul 15, 2020
@oleibman oleibman deleted the staticreset branch November 2, 2020 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants