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

JSON formatter "embeddings" node changed to "embedding" #1235

Closed
BuzMack opened this Issue Oct 2, 2017 · 11 comments

Comments

Projects
None yet
7 participants
@BuzMack
Copy link

BuzMack commented Oct 2, 2017

Summary

Current JSON formatter outputs the embeddings node as "embedding". Version 1.2.5 and earlier had this node as "embeddings"

Expected Behavior

In the JSON formatted output file, I'm expecting to see this:

"embeddings": [
    {
        "mime_type": "image/url",
        "data": "aHR0cDovL2xvY2FsaG9zdC9zdGF0aWMvbG9nby5wbmc="
    }
]

Current Behavior

Current output is as follows:

"embedding": [
    {
        "mime_type": "image/url",
        "data": "aHR0cDovL2xvY2FsaG9zdC9zdGF0aWMvbG9nby5wbmc="
    }
]

Possible Solution

Possibly a typo during the rewrite to 2.0.0?

Context & Motivation

This affects reports generated by cucumber reporting project: https://github.com/damianszczepanik/cucumber-reporting

Basically any attachments made during a test are present in the resulting JSON file, however the reporting tool is expecting to find them inside a node named "embeddings", and the resulting output has no attachments at all.

@BuzMack

This comment has been minimized.

Copy link
Author

BuzMack commented Oct 2, 2017

I suspect the simplest solution is to change JSONFormatter.java method addEmbeddingToHookMap (lines 292-298) which currently look like this:

    private void addEmbeddingToHookMap(byte[] data, String mimeType) {
        if (!currentStepOrHookMap.containsKey("embedding")) {
            currentStepOrHookMap.put("embedding", new ArrayList<Map<String, Object>>());
        }
        Map<String, Object> embedMap = createEmbeddingMap(data, mimeType);
        ((List<Map<String, Object>>)currentStepOrHookMap.get("embedding")).add(embedMap);
    }

Change to match this instead:

    private void addEmbeddingToHookMap(byte[] data, String mimeType) {
        if (!currentStepOrHookMap.containsKey("embeddings")) {
            currentStepOrHookMap.put("embeddings", new ArrayList<Map<String, Object>>());
        }
        Map<String, Object> embedMap = createEmbeddingMap(data, mimeType);
        ((List<Map<String, Object>>)currentStepOrHookMap.get("embeddings")).add(embedMap);
    }
@damianszczepanik

This comment has been minimized.

Copy link

damianszczepanik commented Oct 4, 2017

Looks like someone has changed it and by that change the backwards compatibility was broken. Also while it contains array I don't know why someone has changed name of this node.

@gauravkarvir

This comment has been minimized.

Copy link

gauravkarvir commented Oct 24, 2017

Until this is fixed and a new release version is avaialble, I have created custom class FixScreenShotEmbedder which replaces the json node name from embedding to embeddings, execution is triggered by maven exec plugin
<plugin> <groupId>org.codehaus.mojo</groupId> <artifactId>exec-maven-plugin</artifactId> <version>${mojo-exec-maven-plugin}</version> <executions> <execution> <phase>test</phase> <goals> <goal>java</goal> </goals> </execution> </executions> <configuration> <mainClass>com.gk.support.FixScreenShotEmbedder</mainClass> <classpathScope>test</classpathScope> <arguments> <argument>${project.build.directory}/cucumber</argument> </arguments> </configuration> </plugin>

@mlvandijk

This comment has been minimized.

Copy link
Member

mlvandijk commented Oct 24, 2017

@gauravkarvir You can also use the latest SNAPSHOT version, as described here: https://cucumber.io/docs/reference/jvm#installation

@gauravkarvir

This comment has been minimized.

Copy link

gauravkarvir commented Oct 24, 2017

Yeah did try that, Unfortunately Snapshot releases are blocked by IT team. Hence will wait for the new release version to be available, thanks anyways...

@mlvandijk

This comment has been minimized.

Copy link
Member

mlvandijk commented Oct 24, 2017

Ah, too bad...

@Misterhex

This comment has been minimized.

Copy link

Misterhex commented Feb 9, 2018

looks like current snapshot version is still outputting as "embedding" instead of "embeddings".

@mpkorstanje

This comment has been minimized.

Copy link
Contributor

mpkorstanje commented Feb 9, 2018

@mlvandijk

This comment has been minimized.

Copy link
Member

mlvandijk commented Feb 10, 2018

@Misterhex Not sure which version you are using, but the code says "embeddings".

@brasmusson

This comment has been minimized.

Copy link
Contributor

brasmusson commented Feb 10, 2018

looks like current snapshot version

The current snapshot is version 2.3.2-SNAPSHOT, and it is outputting "embeddings".

@lock

This comment has been minimized.

Copy link

lock bot commented Feb 10, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Feb 10, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.