Skip to content

Fix samples/tests abnormal scenes #3219

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
Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Fix samples/tests abnormal scenes #3219

wants to merge 9 commits into from

Conversation

@jasonbu
Copy link
Contributor

@jasonbu jasonbu commented Nov 25, 2025 •
edited
Loading

Summary

Fix fs date case did not strict enough, some times , the test cross minutes, and will make case abort.
Fix some case break if we enable Net & local socket & rpmsg socket, but did not enable iNet and ipv6
Fix roundrobin always succ. and add SMP compatbile

Impact

None

Testing

CI-test, internal board & test

@jasonbu
kernel/syscall: fix required INET may case no support
09b4c9e 
We may possible enable socket and no ipv4

Signed-off-by: buxiasen <buxiasen@xiaomi.com>
@jasonbu
fs_stat_test: fix fs date case not strict enough
d95ce24 
use utc diff should prefer and more strict.

Signed-off-by: buxiasen <buxiasen@xiaomi.com>
@jasonbu
testing/pm_smp: add more delay, avoid idle canot run one cycle
a3ee39d 
If the host performance low, possible not able to enter idle thread,
cause case break.

Signed-off-by: buxiasen <buxiasen@xiaomi.com>
@jasonbu jasonbu force-pushed the tests branch 2 times, most recently from cab76dc to 6503dfd Compare November 25, 2025 06:43
@jasonbu jasonbu marked this pull request as ready for review November 25, 2025 06:43
xiaoxiang781216
xiaoxiang781216 reviewed Nov 25, 2025
View reviewed changes
@jasonbu
fb: fix no mmap with get_pinfo, kernel build with data abort
2b04262 
When BUILD_KERNEL, will case userspace access the kernel address and
abort.

Signed-off-by: buxiasen <buxiasen@xiaomi.com>
@jasonbu
iperf: compatible work with no IPv4
1eb019a 
We possible want to use iperf to test rpmsg socket etc.

Signed-off-by: buxiasen <buxiasen@xiaomi.com>
@jasonbu
cmake/process-spawn: remove if not required align with Makefile
8841299 
We already fixed the process-spawn.c in Makefile, should align to cmake

Signed-off-by: buxiasen <buxiasen@xiaomi.com>
@jasonbu
drivertest_uart: uart device open should use cfmakeraw to ensure raw.
80cbcf8 
reference: https://www.man7.org/linux/man-pages/man3/termios.3.html

Signed-off-by: buxiasen <buxiasen@xiaomi.com>
@jasonbu
apps/ostest/roundrobin: add fail detect and print
8571ba1 
Before patch we never detect the roundrobin fail.
After patch use a array to detect if the calculation swapped when
processing.

Signed-off-by: buxiasen <buxiasen@xiaomi.com>
@jasonbu
testing/ostest/roundrobin: add compatible for SMP
fbf3bfd 
Fix if SMP cause calculation run in multi-core cause case break.

Signed-off-by: buxiasen <buxiasen@xiaomi.com>
@linguini1
Copy link
Contributor

linguini1 commented Nov 26, 2025

Is there any chance you can split this into multiple PRs? 9 commits is a lot. Also, can you please provide more detail about what the internal CI tests?

There is an impact of this PR if you are modifying code. Please list what applications/tests are impacted by this change.

@jasonbu
Copy link
Contributor Author

jasonbu commented Nov 27, 2025

Is there any chance you can split this into multiple PRs? 9 commits is a lot. Also, can you please provide more detail about what the internal CI tests?

There is an impact of this PR if you are modifying code. Please list what applications/tests are impacted by this change.

Merge patch into one pr aim to save the CI resource. if split too much will cause more CI actions.
If we create pr for specific topic, I agree with we should only include relative patchs.

Every commit is described in commit msg, maybe don't have to copy all from commit msg to github pr?

About internal CI, we are not only one codebase from github. when we using these code for project, patch already included and tested.

@raiden00pl
Copy link
Member

raiden00pl commented Nov 27, 2025

@jasonbu You can't assume that if something works in Xiaomi internal repo, it will also work with the upstream. Xiaomi repositories are not synchronized with the upstream. This has led to bugs before. Changes to Apache repositories should be tested with Apache repositories. Splitting PRs into smaller ones is safer because it's easier to do reviews, even if the cost is higher CI usage.

@jasonbu
Copy link
Contributor Author

jasonbu commented Nov 27, 2025

@jasonbu You can't assume that if something works in Xiaomi internal repo, it will also work with the upstream. Xiaomi repositories are not synchronized with the upstream. This has led to bugs before. Changes to Apache repositories should be tested with Apache repositories. Splitting PRs into smaller ones is safer because it's easier to do reviews, even if the cost is higher CI usage.

Sure, we have to re check to ensure no any break when upstream,
as these change a all small and in apps, merged commits into one pr here.

Next pr will make it more clear and more relative and more convenient for reviewer。

@linguini1
Copy link
Contributor

linguini1 commented Nov 27, 2025 •
edited
Loading

Sure, we have to re check to ensure no any break when upstream, as these change a all small and in apps, merged commits into one pr here.

Yes, so can you please tell us what you tested, how you tested and include the log results?

Next pr will make it more clear and more relative and more convenient for reviewer。

No, all PRs need to be clear and convenient for the reviewers. Please update this PR so that it is more clear, as raiden suggested.

I really suggest all these commits be broken into multiple PRs, regardless of CI resources, because you have many unrelated fixes for multiple apps and they will all require their own summary, impact and test description.

linguini1
linguini1 requested changes Nov 27, 2025
View reviewed changes
Copy link
Contributor

@linguini1 linguini1 left a comment

Choose a reason for hiding this comment

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

Needs proper testing, should be split into multiple PRs.

cederom
cederom requested changes Nov 27, 2025
View reviewed changes
Copy link
Contributor

@cederom cederom left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you @jasonbu , please follow https://github.com/apache/nuttx/blob/master/CONTRIBUTING.md as requested by @linguini1 and @raiden00pl already :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cederom cederom cederom requested changes

@linguini1 linguini1 linguini1 requested changes

@xiaoxiang781216 xiaoxiang781216 xiaoxiang781216 approved these changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

Area: Examples Area: Networking Area: System Area: Testing Size: M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jasonbu @linguini1 @raiden00pl @cederom @xiaoxiang781216