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

Test naming changed from 1.8 to 1.9 for floats #25

Closed
TWiStErRob opened this issue Oct 9, 2022 · 6 comments
Closed

Test naming changed from 1.8 to 1.9 for floats #25

TWiStErRob opened this issue Oct 9, 2022 · 6 comments

Comments

@TWiStErRob
Copy link

I had this test:

@RunWith(TestParameterInjector.class)
...

@Test
public void testLightStateOnDevice(
	@TestParameter({"45", "0", "-3", "-6", "-9", "-12", "-15", "-18", "-21"}) float angle,
	@TestParameter Preset preset
) {

I'm using it for screenshot test recording, so the test names between previous and next versions has to match (otherwise hard to compare screenshots :). When updating from TPI 1.8 to 1.9 my PR failed:

java.lang.AssertionError: File src/test/snapshots/images/net.twisterrob.sun.android.logic_SunAngleWidgetViewScreenshotTest_testLightStateOnDevice[45.0,Nice_Preview].png does not exist

when inspecting the golden values on master (1.8) I found:

net.twisterrob.sun.android.logic_SunAngleWidgetViewScreenshotTest_testLightStateOnDevice[45,Nice_Preview].png

Since the only thing changed in the PR was the TPI version number, it's very likely that this had an unintended behavior change:
e390bab#diff-f0f3c84fc90c4da3bd5e1267e7b74d5774029389e86dfe1bba895740b8b886d0R142

@nymanjens
Copy link
Collaborator

Hi!

Thanks for letting me know.

The behavior you are seeing surprises me, because we have a unit test since 2020-08-05 that tests that

    @Test
    public void withFloat(@TestParameter({"1.2", "2"}) float param) {
      testNameToParameterMap.put(testName.getMethodName(), param);
    }

gets the test name

"withFloat[param=2.0]"

which seems to match your use case.

(I can't link to the test because it is removed when exporting to Github due to a dependency on textproto parsing)

I suspect this may be a combination of a change in TestParameterInjector and your JVM being different (e.g. Anroid vs openJDK).

@nymanjens
Copy link
Collaborator

Hmm, scrap that. Even before this library was open sourced, we had a unit test that ran on an Android emulator that checks that @TestParameter({"1.2", "2"}) float param) gives "withFloat[param=2.0]".

Since May 2022, we extended this test with multiple Android versions, starting at SDK version 15.

So that's probably not it either.

@nymanjens
Copy link
Collaborator

it's very likely that this had an unintended behavior change:
e390bab#diff-f0f3c84fc90c4da3bd5e1267e7b74d5774029389e86dfe1bba895740b8b886d0R142

(just reading this now)

It feels like it can't be a coincidence, I agree.

In your case, since parameter names are not present and the value is not an array, it should change value() to valueAsString(), which in turn becomes String.valueOf(value());.

So then String.valueOf(value()) should give a different result as MessageFormat.format("{0}", value()).

From the documentation of MessageFormat, it seems like it uses something like NumberFormat.getInstance(getLocale()).format(argument) to format the float. That could explain why you are seeing a different result than the Google test infrastructure.

If that's the case, I'd argue this is actually an improvement because it makes the result independent of the locale.

@TWiStErRob
Copy link
Author

Ok, I'd say fair evaluation, nice! :)
Would you mind adding a hint in the changelog as this is a "breaking" behavior change (depending on usage of course).

@TWiStErRob
Copy link
Author

Oh, btw my execution is on JVM, not Android devices. Paparazzi is amazing tech!

@nymanjens
Copy link
Collaborator

Would you mind adding a hint in the changelog as this is a "breaking" behavior change (depending on usage of course).

Done in cd15758

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

No branches or pull requests

2 participants