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

tests/net/gcoap_fileserver: Fix failing nightlies #19856

Merged
merged 1 commit into from
Aug 3, 2023
Merged

tests/net/gcoap_fileserver: Fix failing nightlies #19856

merged 1 commit into from
Aug 3, 2023

Conversation

MrKevinWeiss
Copy link
Contributor

@MrKevinWeiss MrKevinWeiss commented Aug 2, 2023 •
edited
Loading

Contribution description

This test has been failing inconsistently in the nightlies and sometimes on unrelated PRs. I was able to reproduce it with murdock and as soon as I added some print statements to the test it went away. Since the issue is something to do with a failure when comparing 2 files after a node sends a file I am guessing that the comparison comes occasionally too fast and the file is not ready.

Maybe there is some more synchronization needed after a ncput but for now just adding a small sleep should prevent this issue.

Testing procedure

Run murdock multiple times or try to get it failing on a local setup then apply the PR...

Issues/PRs references

@MrKevinWeiss MrKevinWeiss added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: full build disable CI build filter labels Aug 2, 2023
@MrKevinWeiss MrKevinWeiss requested a review from kaspar030 as a code owner August 2, 2023 07:02
@MrKevinWeiss
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Aug 2, 2023
@bors
Try #19856:
0097262
@github-actions github-actions bot added the Area: CI Area: Continuous Integration of RIOT components label Aug 2, 2023
@riot-ci
Copy link

riot-ci commented Aug 2, 2023 •
edited
Loading

Murdock results

✔️ PASSED

653bd61 tests/net/gcoap_fileserver: fix race condition

Success Failures Total Runtime
17 0 17 01m:41s

Artifacts

@bors
Copy link
Contributor

bors bot commented Aug 2, 2023

try

Build failed:

@github-actions github-actions bot added the Area: tests Area: tests and testing framework label Aug 2, 2023
@MrKevinWeiss
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Aug 2, 2023
@bors
Try #19856:
e8b11ad
@MrKevinWeiss
Copy link
Contributor Author

bors try

@bors
Copy link
Contributor

bors bot commented Aug 2, 2023

try

Already running a review

@bors
Copy link
Contributor

bors bot commented Aug 2, 2023

try

Build failed:

@MrKevinWeiss
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Aug 2, 2023
@bors
Try #19856:
2934a9f
@bors
Copy link
Contributor

bors bot commented Aug 2, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@MrKevinWeiss
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Aug 2, 2023
@bors
Try #19856:
a87d3f7
@bors
Copy link
Contributor

bors bot commented Aug 2, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@MrKevinWeiss
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Aug 2, 2023
@bors
Try #19856:
255e46b
@bors
Copy link
Contributor

bors bot commented Aug 2, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@MrKevinWeiss
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Aug 2, 2023
@bors
Try #19856:
451c297
@bors
Copy link
Contributor

bors bot commented Aug 2, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@MrKevinWeiss
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Aug 2, 2023
@bors
Try #19856:
d10cb6f
@bors
Copy link
Contributor

bors bot commented Aug 2, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@github-actions github-actions bot removed the Area: CI Area: Continuous Integration of RIOT components label Aug 2, 2023
@MrKevinWeiss MrKevinWeiss changed the title REMOVEME: just testing tests/net/gcoap_fileserver: Fix failing nightlies Aug 2, 2023
@MrKevinWeiss
Copy link
Contributor Author

Here is it with the fix, I will try a few times... Maybe @benpicco or @maribu can take a quick look.

@MrKevinWeiss
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Aug 2, 2023
@bors
Try #19856:
bd53f29
benpicco
benpicco approved these changes Aug 2, 2023
View reviewed changes
Copy link
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

I don't quite understand why it would work (Linux should be aware of it's page cache even when things are not synced to disc yet) but if it fixes things ¯\(ツ)/¯

@MrKevinWeiss
Copy link
Contributor Author

I am not sure if it fixes things I just cannot fail them anymore... I also see that there is a sleep 10 so an extra 0.5 seconds of sleep is probably not too bad (and it is commented)

@MrKevinWeiss
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Aug 2, 2023

🕐 Waiting for PR status (GitHub check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set.

@bors
Copy link
Contributor

bors bot commented Aug 2, 2023

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: full build disable CI build filter labels Aug 2, 2023
@benpicco
Copy link
Contributor

benpicco commented Aug 2, 2023

bors merge

1 similar comment
@MrKevinWeiss
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Aug 3, 2023

Already running a review

@bors
Copy link
Contributor

bors bot commented Aug 3, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit bb9011c into RIOT-OS:master Aug 3, 2023
@MrKevinWeiss MrKevinWeiss deleted the test/gcoap branch August 3, 2023 06:02
@benpicco
Copy link
Contributor

benpicco commented Aug 7, 2023

Looks like this didn't help much 😕

https://ci.riot-os.org/details/3b451741657544d39b8b6e25401fc7d5

@MrKevinWeiss
Copy link
Contributor Author

shoot, flakey failures really makes life hard...

@MrKevinWeiss
Copy link
Contributor Author

MrKevinWeiss commented Aug 7, 2023 •
edited
Loading

Well at least we know it isn't a race condition. What about adding a print on error and retry send if they don't match, just to get some more info on the issue (if it is a toggled bit or missing byte vs missing file might tell us a bit more)?

@MrKevinWeiss MrKevinWeiss added this to the Release 2023.10 milestone Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benpicco benpicco benpicco approved these changes

@kaspar030 kaspar030 Awaiting requested review from kaspar030

@smlng smlng Awaiting requested review from smlng

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@aabadie aabadie Awaiting requested review from aabadie aabadie is a code owner

@MichelRottleuthner MichelRottleuthner Awaiting requested review from MichelRottleuthner MichelRottleuthner is a code owner

@fjmolinas fjmolinas Awaiting requested review from fjmolinas

Assignees
No one assigned
Labels
Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR
Projects
None yet
Milestone
Release 2023.10
Development

Successfully merging this pull request may close these issues.
3 participants
@MrKevinWeiss @riot-ci @benpicco