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

Issues/37 unittests #47

Merged
merged 7 commits into from
Nov 8, 2023
Merged

Issues/37 unittests #47

merged 7 commits into from
Nov 8, 2023

Conversation

tresf
Copy link

@tresf tresf commented Apr 22, 2019

Adds basic read/write unit tests by starting up a virtual com port using socat.

If socat is missing, the test will automatically be skipped.

Partially addresses #37

.travis.yml Outdated Show resolved Hide resolved
@tresf

This comment was marked as outdated.

@bmarwell

This comment was marked as outdated.

@tresf tresf merged commit 644db73 into master Nov 8, 2023
@tresf tresf deleted the issues/37-unittests branch November 8, 2023 21:45
@tresf
Copy link
Author

tresf commented Nov 8, 2023

Merging so that we can have this on master for other useful tests. I believe in the future we may want to split out the threading logic from VirtualPortRule to clearly indicate that it's part of the unit test, not the rule which invokes the test, although I appreciate -- for simplicity's sake in the tests -- why the current design approach was taken. I'm also curious if this type of bootstrapping inside a rule is common for JUnit.

image
On a personal note...

Secondly, I'd like to offer my apologies for initially pushing back on this PR.

  • When I had originally spoken with @scream3r (the original author of JSSC) back when the fork was originally created, he made a great emphasis on the fact that "JSSC" stands for "Java Simple Serial Connector", and as such, "too much tooling" defeats the purpose of keeping it simple.
  • One (in my opinion, strong) point that @scream3r made was that often serial port issues need physical hardware to be tested due to the intricacies of a bad, interrupted or unexpected data.
  • Furthermore, depesite -- personally -- coding Java for most of my career, I'm embarrassingly not very familiar with JUnit (I still, to this date find it extremely difficult to read and understand).
  • Lastly, and mostly unrelated to this PR, but perhaps the project's tenure in general -- maven --I still find to be a bloated mess for such a small project, where most of the time is spent waiting for it to fetch a myriad of dependencies, which become more and more evident as build systems move to immutable OSs (such as in a container). I wholeheartedly dislike the maven build system with every ounce of my being (this has not and likely will not changed) and find myself staring blindly at nearly unreadable XML, when I'd much prefer something like CMAKE, QMAKE, bash, etc (I'd even be happier with ant, tbh).

That said, the maven build system we use is pretty reliable and the junit tests -- although nearly impossible to read the output from -- are the best we have right now, so I'm sorry if this causes pushback where it's not warranted, or perhaps misdirected.

I do still believe that @scream3r's "simple" principal can still apply here whilst still allowing modern features, but I can do that without driving away good contributors, and for that, I'm sorry.

@tresf
Copy link
Author

tresf commented Nov 8, 2023

Sorry, this came in a merge commit, it was intended to be a squash commit.

@tresf tresf mentioned this pull request Nov 10, 2023
@pietrygamat pietrygamat added this to the 2.9.6 milestone Dec 15, 2023
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