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

Suggestion: Introduce DebugInformation #23

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

mmichaelis
Copy link
Contributor

@mmichaelis mmichaelis commented Jul 18, 2016

Overview

I am actually done with a first proposal especially regarding issue #21. Instead of just having actual and expected values only you can now provide a complete set of values which might be useful for debugging purpose.

What I had in mind where especially system tests (and more specifically Web UI tests as they are one of the test types I am most involved in). That's why one of the examples for possible debug information to provide are images.

Issues

This PR changes the complete structure of the exception and addresses also the following issues:

Discussion

The workspace contains a README.md with some discussion elements I will state here again:

Difference ValueDescriptor vs. ValueWrapper

While developing the ValueWrapper it seems at some points that the ValueWrapper could actually replace the ValueDescriptor. It just needs to store identity hashcode and value of toString in the data map.

But: The roles are slightly different. A ValueWrapper is especially responsible for providing a serializable representation of the object so that the ValueDescriptor can try its best to store the value in a convenient manner to be part of the exception description (thus it needs to be
serializable).

The ValueWrapper is especially meant to be used internally within the ValueDescriptor. That is why the ValueDescriptor provides no direct access to the ValueWrapper.

ValueWrapper: Object or Generics?

Should the ValueWrapper return the stored value as object or as generic type? Using Object as return type provides more flexibility to the ValueWrapper. It might for example decide that the deserialized object is of another type than the serialized one. Using a generic type would prevent this feature.

Provide RenderedImageSerializer?

Should the core of OpenTest4J provide a serializer for rendered images? In terms of providing the very core for test assertions it should not be part of it. Nevertheless it is a typical for example for UI
tests what you would provide a screenshot in addition to other debug information.

So we might place it into the tests section to serve as example and as test for the extensibility. Or we might provide a separate module/project with examples for serializers.

Using Java Service Provider Framework?

The advantage of this framework is, that it is at hand without any additional dependencies. And: It provides a good extension point for additional serializers.

But it also raises some questions:

  • Should the DefaultValueSerializer be part of the service configuration and as such overridable?
  • Is it ok just to rely on the classloader order to control the order of serializers?

DebugInformationDefaultKey

I think it is useful to provide some common defaults we agree upon. But what are these defaults? expected, actual and perhaps a mismatchDescription? And is it better to provide it as enum or just plain String constants?

Conclusion

I think the given result will help me in my work with Web UI tests where I need to collect information like:

  • screenshot
  • current DOM structure
  • contents from JavaScript error console

Nonetheless I am happy if I could just bring up some ideas.


I hereby agree to the terms of the Open Test Alliance Contributor License Agreement.

Introducing a more generic way to provide additional information
for debugging upon assertion failures. The DebugInformation is
key-value based and can contain actual any information.

In addition to this extended the ValueWrapper (now renamed to
ValueDescriptor as it *describes* the value) so that it does some
effort to serialize objects applying some strategies. This way we
could also provide images as for example for UI tests.

The current state neither includes documentation, more tests and
misses the commonly used exception for all exceptions provided
inside here.

This approach also addresses issues:

* ota4j-team#20 support for exception creation strategies
* ota4j-team#4 detach AssertionError from exceptions here
* ota4j-team#9 partially, as you the debug information provided here should
  eventually merge into a standard test report.
Experience shows that you cannot even trust toString() of objects.
They sometimes fail with an exception. ValueDescriptor should
be robust against such behavior.
* Introduces DebuggableException as base for all exceptions which should provide
  debug information.
* Makes AssertionFailedException a DebuggableException
  Keeps actual/expected constructors for now.
* MultiFailuresException: Adjust newline to system line.separator
* Repackages DebuggableException-classes
* Introduces test helpers like for serialization and to do assertions on exceptions.
* Refactored ExceptionTestHelper to provide better/easier to understand
  support for testing DebuggableExceptions.
* Introduces test for DebuggableException
* Renames methods of DebugInformation to be similar to Map interface.
* Provide a lot more tests.
* Remove unused methods from AssertionFailedException about actual/expected.
Adjust Javadoc as the statements about actual/expected values are
not valid anymore.
@mmichaelis
Copy link
Contributor Author

😊 Had some trouble because IntelliJ Idea formats in another way than the Eclipse settings. Would be easier to adapt Idea settings than to use the plugin which does not always apply the format.

And at the end it was Idea's "Optimize Imports" which I had enabled for commit which broke the previous change of spotlessApply.

@mmichaelis
Copy link
Contributor Author

PR #25 contains an alternative solution using the AssertionError again within the hierarchy - with the drawback of some duplicated code.

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

Successfully merging this pull request may close these issues.

1 participant