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

fix: remove-pytest_exception_interact hook #881

Merged
merged 12 commits into from
Aug 28, 2024

Conversation

kdoroszko-splunk
Copy link
Contributor

@kdoroszko-splunk kdoroszko-splunk commented Aug 12, 2024

This PR reverts partially changes introduced in this PR:
7cc8880#diff-2699a131e5dfd55b20327a39ec213c806ee847e17b6666faf0a01fe850448f47L558
This change is necessary, since exiting pytest while getting Exception disturbed other tests (fr example UI test https://splunk.atlassian.net/browse/ADDON-72764). Instead of that HEC token is validated by ingesting simple event with it (implemented in this PR: #877).

@kdoroszko-splunk kdoroszko-splunk marked this pull request as ready for review August 23, 2024 13:16
@kdoroszko-splunk kdoroszko-splunk requested a review from a team as a code owner August 23, 2024 13:16
Copy link

@pawel-dabro pawel-dabro left a comment

Choose a reason for hiding this comment

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

I'm trying to understand how this worked before, my impression is that just specifying an invalid token in the fixture was enough to fail some tests and then the assertion checked that this in fact happened. And I'm not clear on why now we need to poke the endpoint with a fake message to trigger the error. If we didn't do that before - where was the exception that satisfied the previous assertion coming from?

pytest_splunk_addon/splunk.py Outdated Show resolved Hide resolved
.github/workflows/build-test-release.yml Outdated Show resolved Hide resolved
@harshilgajera-crest
Copy link
Contributor

harshilgajera-crest commented Aug 28, 2024

Hey @kdoroszko-splunk , can you help me understand, how earlier it was breaking UI tests. Maybe we can get on a call if requried.

@kdoroszko-splunk
Copy link
Contributor Author

Hey @kdoroszko-splunk , can you help me understand, how earlier it was breaking UI tests. Maybe we can get on a call if requried.

Hello @harshilgajera-crest,
It was breaking UI tests because of this parts of the code: https://github.com/splunk/pytest-splunk-addon/pull/881/files#diff-bd63a95fdfaf908e52f87842a005bcb019d56cc3bcb9cf491952b9402c0affef.
There was an Exception appearing while some element on webpage was not found (example comes from MSCS TA):
!!!!!!!!!!!!! _pytest.outcomes Exit: Exiting pytest due to: «ExceptionInfo Exception('Timeout: Message: by=css selector select=[data-test="control-group"][data-name="http_timeout"] input Element not found in the page\n') tblen=19> !!!!!!!!!!!!!
which caused pytest exit after ~10 tests (the whole testset contains ~120 test cases)

So the root cause was that previous solution was to general.

@kdoroszko-splunk kdoroszko-splunk merged commit 19f3967 into develop Aug 28, 2024
20 of 21 checks passed
@kdoroszko-splunk kdoroszko-splunk deleted the fix/remove-pytest_exception_interact-hook branch August 28, 2024 10:45
@github-actions github-actions bot locked and limited conversation to collaborators Aug 28, 2024
@srv-rr-github-token
Copy link
Contributor

🎉 This issue has been resolved in version 5.4.0-beta.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants