Skip to content

Fix #11684: Improve AfterTest() for Jasmine#11957

Merged
christian-bromann merged 13 commits intowebdriverio:mainfrom
tech-dm-klymenko:dmklymenko-improve-afterhook-jasmine
Jan 19, 2024
Merged

Fix #11684: Improve AfterTest() for Jasmine#11957
christian-bromann merged 13 commits intowebdriverio:mainfrom
tech-dm-klymenko:dmklymenko-improve-afterhook-jasmine

Conversation

@tech-dm-klymenko
Copy link
Contributor

Fix for #11684

Proposed changes

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • I have added proper type definitions for new commands (if appropriate)

Further comments

Reviewers: @webdriverio/project-committers

Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

Some comments, great progress!

@tech-dm-klymenko
Copy link
Contributor Author

And yet another unexpected moment came to light. Even if the tests are passed, the variable result remains undefined and does not contain any information for afterHook()

result = await executeAsync.call(this, specFn, retries, specFnArgs, timeout)

@christian-bromann christian-bromann added the PR: Bug Fix 🐛 PRs that contain bug fixes label Jan 5, 2024
Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

@tech-dm-klymenko last request: can we add a smoke test for this to ensure we don't regress in the future. I would recommend to create a new smoke test suite that runs a single (failing) jasmine test and writes the result of the afterTest hook into a file to then verify the result again in the smoke test. Let me know if you have any questions.

@tech-dm-klymenko tech-dm-klymenko changed the title Fix #11684: Improve Afterhook() for Jasmine Fix #11684: Improve AfterTest() for Jasmine Jan 18, 2024
@tech-dm-klymenko
Copy link
Contributor Author

@christian-bromann
The smoke test turned out to be quite complex; however, it not only checks the existence of values but also ensures that we receive the expected structure and that it has not changed (even under the influence of future updates).

What do you think?

Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

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

Amazing work, thanks so much 🙏

fyi: the build is currently failing due to an issue with the component tests, will move ahead and merge as everything before passed just fine.

@christian-bromann christian-bromann merged commit 17c74bd into webdriverio:main Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: Bug Fix 🐛 PRs that contain bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants