writethemfirst / approvals-java Goto Github PK

When computing approval tests, the framework should allow not only to compare single files, but to compare full directories containing several files/folders.

Given a batch method, generating directories and files,
When running the approval tests,
Then the framework should compare all the generated data (files and folders hierarchy) with a reference one, committed along with the source code.

When writting tests for a method with multiple arguments, the framework should provide a way to compute all combinations of possible arguments from values provided by the developer.

Given a method foo(a,b,c) which is tested by our framework,
When a developer is calling Approvals.verifyAll(::foo, aValues, bValues, cValues),
Then the framework should compute all combinations of provided values for all parameters, and run the verification for all those cases.

For example:

Testing a+b with a={1,10,100} and b={2,3} should run all following verifications:

a=1; b=2
a=10; b=2
a=100; b=2
a=1; b=3
a=10; b=3
a=100; b=3

Complete the README file with a nice and clear documentation containing examples of each feature provided by the framework.

All API methods should be illustrated in that documentation.

Requirement: #11

When comparing a generated directory with a reference one, it should be possible for the developer to specify files to be ignored during the comparison.

For a first step, the files to be ignored could be specified as arguments to the test method which is called. Later on, a file similar to .gitignore could be used to simplify the management of those ignored files.

See Legacy implementation (not sure we want to do it the same way)

All commits should lead to Continuous Deployment builds to be triggered and SNAPSHOT versions to be pushed on bintray.

Note:

• The repository location should be added to the documentation for developers to test upcoming releases

Since kdiff3 is a multi-platform program, it would make sense to add it to the list of supported linux reporters, so we have a consistent behaviour across platforms.

I could make a PR

see for examples:

• https://medium.com/@BillyKorando/whats-new-in-junit-5-3-c276eb8507f1
• https://keyholesoftware.com/2018/03/12/fluent-assertions-with-assertj/
• https://github.com/ota4j-team/opentest4j/blob/master/src/test/java/org/opentest4j/MultipleFailuresErrorTests.java
• http://joel-costigliola.github.io/assertj/assertj-core-features-highlight.html#soft-assertions

it's useless and can cause issues when running in docker containers with no write access for instance

Instead of putting all the approved files in src/test/resources in folders named after the FQN of the class and creating files named after each method, annotations should allow the user to customize the location of generated files:

@ApprovalFiles("src/approvals/files")
class TestFoo {
   @ApprovalFile("barFile")
   @Test
   void bar() {
      Approvals.verify(...);
   }
}

Should allow me to have the generated approved and received files located in src/approvals/files/barFile.

When computing the approval tests and facing an empty or different approved file, the framework should allow merge tools to be executed automatically to review and validate the outputs of the program.

This feature should work on any OS, and here's the first list of supported tools to look for:

Windows support

• IntelliJ IDEA diff tools
• GVim
• KDiff 3
• Beyond Compare 3
• Beyond Compare 4
• TortoiseIDiff
• TortoiseMerge
• WinMergeU
• Araxis Merge
• Code Compare

Linux support

• IntelliJ Ultimate IDEA diff tools
• Beyond Compare
• IntelliJ Community IDEA diff tools
• GVim
• KDiff 3

Mac OS X support

• IntelliJ IDEA diff tools
• KDiff 3
• GVim
• DiffMerge
• Beyond Compare
• Kaleidoscope
• p4merge
• tkdiff

See legacy implementation

see https://github.com/approvals/ApprovalTests.Java/blob/master/java/org/approvaltests/reporters/MultiReporter.java

When computing tests for methods with several arguments, approval files should be formatted so all the arguments' names are kept in the file.

Given a method foo(a,b,c) which is tested by our framework, and producing result when called,
When approval tests are computed and the approval files are generated,
Then the produced file should be formatted as follows:

a=x; b=y; c=z; output=result;

So the file remains readable by a human.

When executing a merge tool for reviewing approval files, the framework should first search for a DIFFTOOL environment variable. If it is set, the tool specified by this variable should be executed by default.

Note:

• Documentation has to be written for that feature

I think we could use approvals-java together with something like selenium to extend the concept of approval testing to UI testing.

This might be an Epic rather than a Story ... Possible steps:

• check that approving an image works (probably need to integrate new reporters which support image diff like this one). We'll also need to define what kind of files a reporter supports (Image / Text / ???).
• check it also works on CI (with no software installed, how to do image diff in pure java ?)
• document approving an image
• document how to get an image from a website (using Selenium for instance - SO)
• optionnally document how to get an image from a Swing application (using Java Robot)
• give a full example explaining the approach

Inspiration:

• https://www.infoq.com/news/2018/05/automated-visual-testing
• https://github.com/AppraiseQA/appraise/blob/master/README.md/

When no merge tool can be found on the developer's workstation, the framework should display a custom Swing window allowing to review each entry line by line (with a simple YES/NO button for each line displayed), so the approved file can be reviewed from a graphical tool.

https://sonarcloud.io/

https://codeclimate.com/

https://github.com/dwyl/learn-codeclimate

For easing the framework's usage, the framework's jar (along with source jars, and javadoc jars) should be deployed to Maven Central.

As a first step, we'd like them to be deployed to a Bintray repository, and to JCenter. But Maven Central is a must as well.

We lost it since March 4th: https://codecov.io/gh/WriteThemFirst/approvals-java

docs: http://www.eclemma.org/jacoco/trunk/doc/check-mojo.html

https://github.com/WriteThemFirst/approvals-java/commits/master/.travis.yml

For instance, this error is difficult to fix:

[info]   java.lang.AssertionError: expected: <Success(
[info]   Event(
[info]     "T\ufffdrmilihus Winterthur",
[info]     "https://www.mtgtop8.com/event?e=18427",
[info]     8,
[info]     Sun Jan 28 00:00:00 CET 2018,
[info]     Highlander,
[info]     None
[info]   )
[info] )
[info] > but was: <Success(
[info]   Event(
[info]     "T\ufffdrmilihus Winterthur",
[info]     "https://www.mtgtop8.com/event?e=18427",
[info]     8,
[info]     Sun Jan 28 00:00:00 UTC 2018,
[info]     Highlander,
[info]     None
[info]   )
[info] )>

As per now, the framework relies on a toString call from the object under verification so it can be written in a file.

In some cases, that might not be convenient (if the toString method is already used for something else, or not providing the information you'd like). The framework should allow to specify a formatter function for that.

For example:

Approvals.verify(anObject, { o -> o.aField + " - " + o.anotherField });

Should allow to format the output not calling toString from the provided object, but calling the formatter function instead.

• format Map (order keys, one key/value per line)
• format collections (one entry per line)

When calling Approvals.verify("actual") with an approved file containing "different" and a CommandReporter, the test should fail

I'm using this to choose: https://choosealicense.com/

Hello,

In order to use VSCode diff, you can use this config in approvals.java

/usr/local/bin/code //// --diff %received% %approved%

The Approvals object and all the Approver objects are to be refactored.

Here are some thoughts after reading again the source code:

• Documentation should be updated to state that Approvals is an easy entry point for accessing only default configuration,
• Approvers can be used for advanced configuration of your tests and validations,
• The custom extension should disappear. Using the csv extension might sound appealing (because of opening the files in other editors), but it actually is really confusing and might lead to other issues (ignoring csv in SCMs, etc.) - I can elaborate on that topic, but my opinion is: we can format the file as if it is a csv file, but it needs to remain a .approved file,
• The custom file name isn't a feature we planned yet, and there is more to think about it than just an entry in the builder object, so it should disappear as well (I'm in favor of an agile approach for that: we implement if and only if we need it (on a real use case))
• As far as possible, the source code itself should allow to understand that Approvers are actually just advanced entry points for approvals. Maybe putting them in an advanced package could do the trick?
• Global refactoring (variable names, functions, etc.)
• Javadoc the whole thing
• Review the error messages and align them to all the other error messages
• Remove from the ApprovalFiles class all the things linked to the extensions

On Ubuntu, due to backwards compatibility reasons, there is symbolic link between /usr/bin/X11 and /usr/bin. Programs expect X11 binaries to be in former but Ubuntu places them in the latter.

This is causing the library to fail on Ubuntu with a java.nio.file.FileSystemLoopException.
This causes the library to fail on Ubuntu bases CI servers such as Semaphore CI too.

I'll publish a fix soon. Meanwhile, here's a complete stack trace :

lambda.infrastructure.code.SSPTemplateEngineSpec *** ABORTED ***
   java.lang.ExceptionInInitializerError:
   at com.github.writethemfirst.approvals.reporters.SupportedOs.<clinit>(SupportedOs.java:45)
   at com.github.writethemfirst.approvals.Reporter.<clinit>(Reporter.java:53)
   at com.github.writethemfirst.approvals.approvers.Approver.<init>(Approver.java:78)
   at lambda.infrastructure.TestUtils$Approbation.$init$(TestUtils.scala:27)
   at lambda.infrastructure.code.SSPTemplateEngineSpec.<init>(SSPTemplateEngineSpec.scala:9)
   at sun.reflect.NativeConstructorAccessorImpl.newInstance0(Native Method)
   at sun.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:62)
   at sun.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
   at java.lang.reflect.Constructor.newInstance(Constructor.java:423)
   at java.lang.Class.newInstance(Class.java:442)
   ...
   Cause: java.io.UncheckedIOException: java.nio.file.FileSystemLoopException: /usr/bin/X11
   at java.nio.file.FileTreeIterator.fetchNextIfNeeded(FileTreeIterator.java:88)
   at java.nio.file.FileTreeIterator.hasNext(FileTreeIterator.java:104)
   at java.util.Iterator.forEachRemaining(Iterator.java:115)
   at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
   at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482)
   at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
   at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
   at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
   at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
   at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
   ...
   Cause: java.nio.file.FileSystemLoopException: /usr/bin/X11
   at java.nio.file.FileTreeWalker.visit(FileTreeWalker.java:294)
   at java.nio.file.FileTreeWalker.next(FileTreeWalker.java:372)
   at java.nio.file.FileTreeIterator.fetchNextIfNeeded(FileTreeIterator.java:84)
   at java.nio.file.FileTreeIterator.hasNext(FileTreeIterator.java:104)
   at java.util.Iterator.forEachRemaining(Iterator.java:115)
   at java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
   at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482)
   at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
   at java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
   at java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
   ...

Each commit should lead to the project being built, Unit Tests to be executed, and the Javadoc to be built and verified.

Deployment will be done in another step.