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

Fixed rasterize_html_command #22146

Merged
merged 6 commits into from
Nov 14, 2022
Merged

Conversation

ShacharKidor
Copy link
Contributor

@ShacharKidor ShacharKidor commented Nov 10, 2022

Status

  • In Progress
  • Ready
  • In Hold - (Reason for hold)

Related Issues

fixes: XSUP-18176.

Description

In this PR I fixed the rasterize-html command by changing the input file name to a name including a .html extension.
I also improved the TPB of the integration by adding a test of the rasterize-html command.

In addition I added the 'full_screen' argument to the rasterize-html command.

Screenshots

The results of the rasterize-html command on the same html file before and after the fix:
Before:
35996A1E-C3B6-4C21-98F9-62CE55B33654

After:
F867ED24-C67B-4965-9480-029B22F007B2

Minimum version of Cortex XSOAR

  • 6.0.0
  • 6.1.0
  • 6.2.0
  • 6.5.0

Does it break backward compatibility?

  • Yes
    • Further details:
  • No

Must have

  • Tests
  • Documentation

@ShacharKidor
Copy link
Contributor Author

ShacharKidor commented Nov 10, 2022

Until now, 'Rasterize Test' (TPB) tested the rasterization of html files by running the 'rasterize-email' command only.
In order to test the 'rasterize-html' command, I added a section that saves the HTML body (tested in the 'rasterize-email' command) to a file and runs the 'rasterize-html' command on it.

You can see that before the fix this section had failed:
56508F08-A145-4CDF-B1C5-D339B1676D34

And after the fix the TPB passed successfully:
Rasterize_Test_Thu_Nov_10_2022

@demisto demisto deleted a comment from xsoar-bot Nov 10, 2022
@ShacharKidor ShacharKidor marked this pull request as ready for review November 10, 2022 13:28
@xsoar-bot
Copy link
Contributor

@xsoar-bot
Copy link
Contributor

The following integrations/tests were collected by the CI build but are currently skipped. The collected tests are related to this pull request and might be critical.:

  • Impossible Traveler - Test - reason: Usage limit reached (Issue 38063)

Copy link
Contributor

@JudahSchwartz JudahSchwartz left a comment

Choose a reason for hiding this comment

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

Perfect!

@ShacharKidor
Copy link
Contributor Author

@ShahafBenYakir - Please force merge the PR.
The error in GitLab build is in a test called 'URL Enrichment - Generic v2 - Test' and is related to a problem configuring the VirusTotal - Private API instance.
All Rasterize TPBs have passed successfully.

@ShacharKidor ShacharKidor added the ForceMerge Forcing the merge of the PR despite the build status label Nov 14, 2022
@ShahafBenYakir ShahafBenYakir merged commit 46b568f into master Nov 14, 2022
@ShahafBenYakir ShahafBenYakir deleted the fix_rasterize_html_command branch November 14, 2022 11:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs-approved ForceMerge Forcing the merge of the PR despite the build status
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants