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

Skip test linuxcncrsh until it become more stable. #2824

Merged

Conversation

petterreinholdtsen
Copy link
Collaborator

As can be seen in several test failures on github and discussed in #2739, the linuxcncrsh test is unstable. It is believed to be caused by a race condition. Until the race condition is fixed, I believe it is best to skip the test.

As can be seen in several test failures on github and discussed in
LinuxCNC#2739, the linuxcncrsh test
is unstable.  It is believed to be caused by a race condition.  Until
the race condition is fixed, I believe it is best to skip the test.
@heeplr
Copy link
Contributor

heeplr commented Jan 2, 2024

Rather than disable the test completely, it might be better to just comment out the line triggering the race condition.

Also for the record: It's not linuxcncrsh that is unstable. The race condition existed for a long time but just didn't ever get tested for before I added more tests.

@petterreinholdtsen
Copy link
Collaborator Author

petterreinholdtsen commented Jan 2, 2024 via email

@heeplr
Copy link
Contributor

heeplr commented Jan 2, 2024

I assume your hunt for a proper fix will soon be successful

I wouldn't expect too much since my linuxcnc-todo list is quite long and this issue could be fixed much quicker by someone familiar with milltask innards.
Also I won't spend a single minute anymore with "malformatting" my PRs so the process mentioned in #2760 needs to be completed first.

More testing is good. :)

I absolutely agree. But only if the bugs bubbling up by more testing are actually fixed ;)
Generally I'd suggest that failing tests shouldn't block the test builds, since test builds can contain errors by definition.

@andypugh
Copy link
Collaborator

andypugh commented Jan 7, 2024

This test is still failing sometimes, so whilst agreeing with what you say, I will merge this.

@andypugh andypugh merged commit 0487cb3 into LinuxCNC:master Jan 7, 2024
11 checks passed
@heeplr
Copy link
Contributor

heeplr commented Feb 13, 2024

Still would be wiser to just disable the failing commands inside the test instead of disabling the complete test.

(I don't really care since I still think there should be -dev/-test builds that keep track of failing tests but without blocking development of other parts of the code. But I guess the underlying bug won't be fixed anytime soon and no one is using linuxcncrsh anyway, so I'm good.)

@petterreinholdtsen petterreinholdtsen deleted the tests-linuxcncrsh-skip branch July 23, 2024 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants