Skip to content
This repository has been archived by the owner on Aug 28, 2022. It is now read-only.

Use describe names for the suite name #198

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

Conversation

alexneo2003
Copy link

added features requested at issue #179
image

@alexneo2003
Copy link
Author

@ryparker can you review PR

@ryparker ryparker self-requested a review June 14, 2021 17:41
@ryparker
Copy link
Owner

Thanks for the PR. I'll make some time this weekend to follow up on this PR and other issues. I'm planning a move to Seattle so time has been limited. If anyone is interested in being added as a maintainer for this project LMK.

Thanks again @alexneo2003

@ryparker ryparker added the enhancement New feature or request label Jun 14, 2021
@ryparker ryparker linked an issue Jun 14, 2021 that may be closed by this pull request
Copy link
Owner

@ryparker ryparker left a comment

Choose a reason for hiding this comment

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

Support for describe blocks is definitely going to improve adoptability of this project however we don't want to break existing support for tests that do not use describe.

Going to deny this implementation since it breaks functionality but I left comments on ideas of how we should implement this.

I appreciate your willingness to contribute to the project. Look forward to hearing your thoughts.

allure.attachment('HTML attachment', '<div><p>This is an HTML doc</p></div', ContentType.HTML);

expect(1 + 2).toBe(3);
});

Copy link
Owner

@ryparker ryparker Jun 21, 2021

Choose a reason for hiding this comment

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

These tests are still valuable for use cases where users have tests that do not use describe to wrap their tests. We should keep these as is and instead add new tests that wrap them with describe


if (suites.length > 0) {
currentTest.addLabel(LabelName.SUITE, suites.join(' > '));
private addSuiteLabelsToTestCase(currentTest: AllureTest, parent: jest.Circus.DescribeBlock): AllureTest {
Copy link
Owner

@ryparker ryparker Jun 21, 2021

Choose a reason for hiding this comment

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

After testing this with the original tests by running yarn test and then allure serve I noticed that this new implementation does not support tests that are not wrapped in describe which would be breaking current functionality. We should be able to support tests that are wrapped either with or without the describe blocks.

I also noticed that the test file name is not the highest layer of the tests report. To match other Allure library implementations this would be something we should support. Especially for cases where there are multiple describe blocks with the same name.

One interesting way i've seen this implemented is in the Allure Jasmine library. They solve this by collecting the describe layers via Jasmine's suiteStarted and suiteDone hooks. The equivalent of this hook would be the start_describe_definition and finish_describe_definition jest environment events.

Something like this could allow us to detect wether the test is in fact wrapped in a describe block. Then we can collect the layers of describe block. Then once the finish_describe_definition is called for that block we would remove it from the collection. All while supporting nested describe blocks, and not effecting current implementation that supports tests without describe blocks.

Copy link
Author

@alexneo2003 alexneo2003 Jun 22, 2021

Choose a reason for hiding this comment

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

file name is not the highest layer of the tests report

for example, I have very long spec files and for convenience I divide them into several files, but the tests in these files have a common describe and in the report I would like to see all these tests in one describe group

Copy link
Author

Choose a reason for hiding this comment

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

sorry, but my knowledge is probably not enough to implement this in the way you suggest using the start_describe_definition and finish_describe_definition events))
i'm trying to combine mine implementation with youth
i'm added extra check for tests without describe and if it exists - use old logic with file path

	private addSuiteLabelsToTestCase(currentTest: AllureTest, parent: jest.Circus.DescribeBlock, testPath: string): AllureTest {
		const isWindows = os.type() === 'Windows_NT';
		const pathDelimiter = isWindows ? '\\' : '/';
		const pathsArray = testPath.split(pathDelimiter);

		const [parentSuite, ...suites] = pathsArray;
		const subSuite = suites.pop();

		if ((parent?.parent as any)?.parent?.parent && (parent?.parent as any)?.parent?.parent.name === 'ROOT_DESCRIBE_BLOCK') {
			currentTest.addLabel(LabelName.PARENT_SUITE, (parent.parent as any).parent.name);
			currentTest.addLabel(LabelName.SUITE, (parent.parent as any).name);
			currentTest.addLabel(LabelName.SUB_SUITE, parent.name);
		} else if ((parent?.parent as any)?.parent && (parent?.parent as any)?.parent.name === 'ROOT_DESCRIBE_BLOCK') {
			currentTest.addLabel(LabelName.PARENT_SUITE, (parent.parent as any).name);
			currentTest.addLabel(LabelName.SUITE, (parent as any).name);
		} else if ((parent as any)?.parent && (parent as any)?.parent.name === 'ROOT_DESCRIBE_BLOCK') {
			currentTest.addLabel(LabelName.PARENT_SUITE, (parent as any).name);
		} else {
			if (parentSuite) {
				currentTest.addLabel(LabelName.PARENT_SUITE, parentSuite);
				currentTest.addLabel(LabelName.PACKAGE, parentSuite);
			}
			if (suites.length > 0) {
				currentTest.addLabel(LabelName.SUITE, suites.join(' > '));
			}
			if (subSuite) {
				currentTest.addLabel(LabelName.SUB_SUITE, subSuite);
			}
		}
		 
		return currentTest;
	}

it will look like
image
can you check it and if this option is suitable I will make a commit

Copy link
Author

Choose a reason for hiding this comment

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

@ryparker can you take me any feedback

@sridharreddysuram123
Copy link

sridharreddysuram123 commented Jul 16, 2021

HI @ryparker /@alexneo2003 any update about this fix? By when this feature will be available?
Thank you!

@alexneo2003
Copy link
Author

@ryparker answer me please

@alexneo2003
Copy link
Author

Hi all
Let him forgive me)
I'm publishing unmerged changes as scoped package.
To use it

yarn add @alex_neo/jest-circus-allure-environment

jest.config.js

 testEnvironment: '@alex_neo/jest-circus-allure-environment',

@ryparker ryparker removed their request for review March 28, 2024 17:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use describe names for the suite name
3 participants