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.

Sign up for GitHub

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

Jump to bottom

feat(modem): host test support of the latest ESP-IDF release (IDF-7828) #454

Merged
merged 2 commits into from
Mar 14, 2024
Merged

feat(modem): host test support of the latest ESP-IDF release (IDF-7828) #454

merged 2 commits into from
Mar 14, 2024

Conversation

gabsuren
Copy link
Contributor

@gabsuren gabsuren commented Dec 7, 2023

No description provided.

@gabsuren gabsuren added the modem label Dec 7, 2023
@gabsuren gabsuren force-pushed the modem/latest_version branch 3 times, most recently from c9afe4a to 114e1cb Compare December 7, 2023 16:48
@gabsuren gabsuren requested a review from david-cermak December 7, 2023 17:01
david-cermak
david-cermak reviewed Dec 8, 2023
View reviewed changes
Copy link
Collaborator

@david-cermak david-cermak left a comment

Choose a reason for hiding this comment

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

LGTM in general.

It only seems like the tests were not executed?
https://github.com/espressif/esp-protocols/actions/runs/7131318373/job/19419621023?pr=454#step:6:386

compared to the master pipeline:
https://github.com/espressif/esp-protocols/actions/runs/7112254051/job/19361852029#step:6:321

the new test has no output, is it because of the new catch?

@gabsuren gabsuren requested review from david-cermak and removed request for david-cermak January 8, 2024 12:41
@gabsuren gabsuren force-pushed the modem/latest_version branch 2 times, most recently from c7f8fc0 to c4d6b50 Compare January 10, 2024 16:07
@gabsuren
Copy link
Contributor Author

LGTM in general.

It only seems like the tests were not executed? https://github.com/espressif/esp-protocols/actions/runs/7131318373/job/19419621023?pr=454#step:6:386

compared to the master pipeline: https://github.com/espressif/esp-protocols/actions/runs/7112254051/job/19361852029#step:6:321

the new test has no output, is it because of the new catch?

Yes in new Catch2 when the result is being redirected with -r option in junit.xml file it doesn't show the output.
I tried to run in without -r, and it showed the amount of tests pass/fail
https://github.com/gabsuren/esp-protocols-1/actions/runs/7474518733/job/20340845490#step:6:408

@gabsuren gabsuren requested a review from david-cermak January 15, 2024 11:15
@david-cermak
Copy link
Collaborator

Yes in new Catch2 when the result is being redirected with -r option in junit.xml file it doesn't show the output. I tried to run in without -r, and it showed the amount of tests pass/fail https://github.com/gabsuren/esp-protocols-1/actions/runs/7474518733/job/20340845490#step:6:408

LGTM if the output is captured in junit.xml, could you please post a link?

@gabsuren gabsuren force-pushed the modem/latest_version branch 2 times, most recently from d9417e0 to ba008fe Compare January 26, 2024 09:45
@gabsuren
Copy link
Contributor Author

Yes in new Catch2 when the result is being redirected with -r option in junit.xml file it doesn't show the output. I tried to run in without -r, and it showed the amount of tests pass/fail https://github.com/gabsuren/esp-protocols-1/actions/runs/7474518733/job/20340845490#step:6:408

LGTM if the output is captured in junit.xml, could you please post a link?

https://github.com/espressif/esp-protocols/actions/runs/7666426164/job/20894215662#step:6:413

@gabsuren gabsuren force-pushed the modem/latest_version branch 2 times, most recently from eeb99fe to 619a784 Compare January 26, 2024 10:02
@gabsuren
Copy link
Contributor Author

gabsuren commented Jan 29, 2024 •
edited
Loading

@david-cermak I added --reporter JUnit::out=result-junit.xml --reporter console::out=-::colour-mode=ansi flags in case if we want to see the output and although redirect it to the file

https://github.com/espressif/esp-protocols/pull/454/files#diff-0e1a184c6942ac42834330ce6f9261da0dbd49d411e5f7847a38ae90fd3f8958R66:~:text=./build/%24%7B%7Binputs.app_name%7D%7D.elf%20%2D%2Dreporter%20JUnit%3A%3Aout%3Dresult%2Djunit.xml%20%2D%2Dreporter%20console%3A%3Aout%3D%2D%3A%3Acolour%2Dmode%3Dansi

The log is here
https://github.com/espressif/esp-protocols/actions/runs/7666426164/job/20894215662#step:6:413

@gabsuren gabsuren force-pushed the modem/latest_version branch 7 times, most recently from e569dee to f7db79a Compare January 31, 2024 11:48
david-cermak
david-cermak reviewed Feb 5, 2024
View reviewed changes
Copy link
Collaborator

@david-cermak david-cermak left a comment

Choose a reason for hiding this comment

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

LGTM otherwise

@gabsuren gabsuren force-pushed the modem/latest_version branch from 64981c9 to 4cb6fb3 Compare February 5, 2024 09:10
@gabsuren gabsuren removed mdns labels Feb 5, 2024
@gabsuren gabsuren force-pushed the modem/latest_version branch 2 times, most recently from e29fdf2 to 54bd546 Compare February 5, 2024 10:34
@gabsuren gabsuren requested a review from david-cermak February 5, 2024 10:35
@gabsuren gabsuren force-pushed the modem/latest_version branch 2 times, most recently from 1cb51dc to 229217a Compare February 19, 2024 09:19
@gabsuren
Copy link
Contributor Author

@david-cermak PTAL on your first convenience.

@gabsuren gabsuren force-pushed the modem/latest_version branch from 229217a to 5f51d48 Compare March 11, 2024 11:38
@gabsuren gabsuren force-pushed the modem/latest_version branch 3 times, most recently from f508360 to a8d9edf Compare March 12, 2024 08:20
@gabsuren gabsuren force-pushed the modem/latest_version branch from a8d9edf to 2cc974c Compare March 12, 2024 10:16
@gabsuren gabsuren force-pushed the modem/latest_version branch from 2cc974c to a23a002 Compare March 12, 2024 10:22
@gabsuren gabsuren requested a review from david-cermak March 12, 2024 10:25
david-cermak
david-cermak reviewed Mar 13, 2024
View reviewed changes
.github/workflows/run-host-tests.yml
@@ -51,14 +57,19 @@ jobs:
# The sdkconfig.ci.linux specifies Linux as the build target with apropriate settings.
cp sdkconfig.ci.linux sdkconfig.defaults
idf.py build
./build/${{inputs.app_name}}.elf -r junit -o junit.xml
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a reusable action, correct? Wouldn't we miss the junit outputs for the websocket and mdns host tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@david-cermak good point. I will modify and fix it in another PR. I'll address and resolve it in a subsequent pull request. As today I noticed a problem with the package upgrade for the mDNS test, but I believe the second commit in this pull request will fix the issue.
Thank you for your review!

david-cermak
david-cermak approved these changes Mar 13, 2024
View reviewed changes
Copy link
Collaborator

@david-cermak david-cermak left a comment

Choose a reason for hiding this comment

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

LGTM, just one last final question. It's okay to just run the related tests to see if it didn't break anything...

@gabsuren gabsuren merged commit 438449f into espressif:master Mar 14, 2024
198 checks passed
@gabsuren gabsuren deleted the modem/latest_version branch July 10, 2024 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@david-cermak david-cermak david-cermak approved these changes

@euripedesrocha euripedesrocha euripedesrocha approved these changes

Assignees

@gabsuren gabsuren

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gabsuren @david-cermak @euripedesrocha @suren-gabrielyan-espressif