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

feat(allure-cypress): impose some limits on step parameter values created from Cypress command arguments #1127

Merged
merged 10 commits into from
Sep 6, 2024

Conversation

delatrie
Copy link
Collaborator

@delatrie delatrie commented Aug 30, 2024

Context

Allure Cypress converts Cypress commands to Allure steps. Command argument values are converted to Allure step parameter values:

  • strings are left unmodified,
  • values of other types are converted with JSON.stringify(value, null, 2).

The drawbacks:

  • A TypeError: Converting circular structure to JSON is thrown if the value is an object with circular references. This is often the case when using Cypress commands with complex UI objects like Vue components, etc. See some examples in issue allure-cypress : getting error - 'Converting circular structure to JSON' #1070.
  • Complex objects and long strings/arrays can easily blow the report to ridiculous sizes

serialize enhancements

We have an existing function serialize defined in allure-js-commons/sdk/reporter/utils, which behaves similarly to what Allure Cypress does with Cypress step arguments. This PR:

  1. Moves it to allure-js-commons/sdk/utils to make it usable from environments other than Node.js.
  2. Adds an optional argument, which, if given, must be an object with two optional properties: maxDepth, maxLength. The value of each property must be an integer number greater than or equal to 0 (which is the default). The value of zero means the property doesn't affect serialize.
  3. Implements the following constraints imposed on the result of serialize:
    • If maxDepth is given and is greater than 0, properties that create nesting levels beyond that limit are precluded from serialization.
    • If maxLength is given and is greater than 0, and if the length of the resulting string is greater than that value, the remaining part of the string is replaced with "...".

Examples

const obj1 = {};
obj1.ref = obj1;
serialize(obj1); // returns "{}"

const obj2 = {
  // this is nesting level 1
  foo: {
    // this is nesting level 2
    bar: {
      // this is nesting level 3
      /* ... */ 
    },
    baz: "Lorem ipsum",
  },
};
serialize(obj2, { maxDepth: 2 }); // returns '{"foo":{"bar":"Lorem ipsum"}}'

serialize({ foo: "Lorem ipsum" }, { maxLength: 10 }); // returns '{"foo":"Lo...'

allure-cypress now uses serialize to convert Cypress command arguments to strings.

Configuration changes

The PR introduces two new configuration properties maxArgumentLength and maxArgumentDepth to customize the abovementioned limits for allure-cypress. The properties are grouped under the stepsFromCommands option:

import { defineConfig } from "cypress";
import { allureCypress } from "allure-cypress/reporter";

export default defineConfig({
  e2e: {
    setupNodeEvents: (on, config) => {
      allureCypress(on, config, {
        stepsFromCommands: {
          maxArgumentLength: 64,
          maxArgumentDepth: 5,
        },
      });

      return config;
    },
    // other Cypress options
  },
});

The default values are 128 (for maxArgumentLength) and 3 (for maxArgumentDepth).

Fixes #1070

Other minor changes

  • improve maps and sets support by serialize: now they may nest in other objects and contain circular references, which are properly removed upon serialization. The values are serialized as arrays instead of the "[object Map]" and "[object Set]" literals.
  • document why the test plan feature may not work (because of the missing 2nd argument of allureCypress)

Potential future changes

Checklist

@delatrie delatrie added the type:new feature New feature or request label Aug 30, 2024
@delatrie delatrie requested a review from epszaw August 30, 2024 12:58
This will allow using 'serialize' from environments other than Node.js.
allure-cypress now uses the new implementation of allure-js-commons'es
serialize instead of its own version.
@github-actions github-actions bot added theme:api Javascript API related issue theme:mocha Mocha related issue labels Sep 3, 2024
Copy link
Member

@epszaw epszaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job here! I noticed nothing serious, so if you want to fix nothing – just skip the comments.

packages/allure-cypress/src/model.ts Show resolved Hide resolved
packages/allure-cypress/src/reporter.ts Outdated Show resolved Hide resolved
packages/allure-cypress/src/runtime.ts Outdated Show resolved Hide resolved
packages/allure-cypress/src/utils.ts Outdated Show resolved Hide resolved
@delatrie delatrie merged commit 5ac84d4 into main Sep 6, 2024
10 checks passed
@delatrie delatrie deleted the issue1070-cypress-cmd-json branch September 6, 2024 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme:api Javascript API related issue theme:cypress theme:mocha Mocha related issue type:new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

allure-cypress : getting error - 'Converting circular structure to JSON'
2 participants