Allow code inside a unit test to know it is inside a test

[mosuem]

Problem

I18n data is updated periodically, which leads to unit tests failing if they implicitly or explicitly call any i18n methods such as number formatting where the values can change.

To counteract these brittle tests, a good method would be to return a fixed string in tests, e. g. (in pseudocode)

// In production
NumberFormat(decimalPlaces: 2).format(3.1415) == '3.14'
// In tests
NumberFormat(decimalPlaces: 2).format(3.1415) == '3.1415 decimalPlaces: 2' // or some similar fixed string.

Goals

This requires for the formatting function to

1. know it is being executed in a test
2. bypass the restriction if one would actually want to test formatting.

Current state

1. is technically possible by checking for the '#test.declarer' Zone value:

if (Zone.current[#test.declarer] != null) {
    return '$number $formattingArgs';
}

but 2. is not.

Possible Solutions

Option 1: Make the tags public, allowing to tag a test as allowFormatting, and checking for the tag to be present. Is hacky and introduces a dependency on package:test in the i18n package.

Option 2: Define a "formatting zone" in the i18n package, have package:test run tests in that formatting zone. Introduces a dependency on the i18n package in package:test, and pins the formatting zone with that particular package.

Option 3: Same as 2, but with the formatting zone defined in a third package independent from the i18n package and package:test, which would both have a dependency to this third package. This makes this formatting zone independent of a particular i18n package, and of i18n in general.

[jakemac53]

Option 1 is a non-starter - you don't want test dependencies leaking into normal apps.

I would be pretty opposed to options 2/3 as well, you can imagine similar requests coming from any number of other packages. In isolation it seems not that bad but it easily becomes bad once you go down that path.

I am not quite fully understanding the issue though, when you say "I18n data is updated periodically" what do you mean by that? Is this in the middle of a test? If the test is expecting a page to look a certain way, wouldn't you want to know if the format changed?

[mosuem]

I18n data means the CLDR data set, which is released twice per year. It introduces changes which will in many cases change the output of functions depending on it. Therefore, tests relying on the output of i18n functions are inherently brittle.

If the test is expecting a page to look a certain way, wouldn't you want to know if the format changed?

If you really want to see the output of formatting, you should be able to. That's why I would like to have some form of flag which can allow formatting in testing in these rare circumstances.

I am fine with having to decide on whether to accept similar requests in the future, seems like a small price to pay to me ;)

[jakemac53]

If a test doesn't care about the exact output, then it shouldn't be hard coding the string in the expectation, it should instead just use the actual function to generate the expected output?

For screenshot/golden tests that gets harder, but those types of tests are usually able to be automatically updated.

[jakemac53]

Is it possible to just use a combination of something like the current strategy (but maybe more explicit) plus an intl zone that allows you to opt out of that behavior? I don't think we actually need to add any explicit coupling here or new dependencies. intl should be able to do something all on its own?

[jakemac53]

(fwiw I wouldn't be opposed to some more explicit sort of zone variable or something which would allow you to ask if you were inside a test, the declarer thing is definitely a bit of a hack)

[mosuem]

If a test doesn't care about the exact output, then it shouldn't be hard coding the string in the expectation, it should instead just use the actual function to generate the expected output?

For screenshot/golden tests that gets harder, but those types of tests are usually able to be automatically updated.

Yes, those are both true. It is hard to enforce this from the outside, so the safest bet is to not format in tests by default.

Is it possible to just use a combination of something like the current strategy (but maybe more explicit) plus an intl zone that allows you to opt out of that behavior? I don't think we actually need to add any explicit coupling here or new dependencies. intl should be able to do something all on its own?

What do you mean with current strategy? How could intl do something on its own - without knowing if it is in a test, and having means to allow i18n nevertheless?

[jakemac53]

What do you mean with current strategy? How could intl do something on its own - without knowing if it is in a test, and having means to allow i18n nevertheless?

I mean using the if (Zone.current[#test.declarer] != null) { hack - or possibly a more supported thing that achieves the same result. For instance a zone variable we would set called isInTestZone or something.

Intl would then also have a similar zone variable it could set, which would override the default behavior of checking for that zone configuration, instead using some explicit zone configuration.

[mosuem]

Yes, checking for some zone variable set by package:test would work, but it would be a bit hacky as it introduces an invisible dependency between the i18n package and package:test, which seems brittle.

For the problem of overriding the don't-format-in-tests rule, I am not sure I understand what you mean. Should the i18n package run all of its methods in a zone, and allow for some zone value to be wired through from a parameter set by the user?

Something like

  test('test name', () {
    NumberFormat.compact(allowFormattingInTest: true).format(100);
  });

? This seems a bit clunky to me; I would rather have

 @Tags(['allowFormatting'])
 
 ...

  test('test name', () {
    NumberFormat.compact().format(100);
  });

or

 test('test name', () {
    NumberFormat.compact().format(100);
  },
  allowFormatting: true,
 );

As this is more of a test property, which I would rather not have leaking into a i18n library interface.

[jakemac53]

There are a lot of potential ways to set up this zone, but it doesn't need to exist on any of the normal APIs, it would typically be a separate API, so something like this as an example API:

T withoutFormatting<T>(T Function() callback);

Users would use it like this:

test('test name', () {
  withoutFormatting(() { // If this callback was async, you would want to await this line
    NumberFormat.compact().format(100);
  });
});

Users would have to do this inside each test function body, because of how package:test uses zones, but its doable. You could also create a wrapper around the test function if you desired:

// TODO: support all params `test` supports
void testWithoutFormatting(String name, void Function() testFn) {
  test(name, () => withoutFormatting(testFn));
};

And people could use that instead of test, which is a bit less boilerplate. Note that this may screw up IDE integration a bit though.

[natebosch]

Looks like another use case for https://github.com/dart-lang/test/issues/377

[mosuem]

The zone should rather be

T withFormatting<T>(T Function() callback);

, as withoutFormatting should be the default. But that sounds good!

Looks like another use case for https://github.com/dart-lang/test/issues/377

Yes, although defining it yourself is not a lot of work here.

The implementation in the i18n library now looks like this.

import 'dart:async';

bool isInTest() {
  if (Zone.current[#test.declarer] != null &&
      !Zone.current[#test.allowFormatting]) {
    return true;
  } else {
    return false;
  }
}

T withFormatting<T>(T Function() callback) => runZoned(callback, zoneValues: {#test.allowFormatting: true});