-
Notifications
You must be signed in to change notification settings - Fork 242
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
Fix Autobahn Scripts #165
Fix Autobahn Scripts #165
Conversation
5d14e85
to
6b59e70
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just some technicalities.
BTW, it seems that GH Actions work well - we can as well remove Travis, if it has been completely superseded by GH Actions.
According to the documentation from the Autobahn Suite GitHub page, it seems like they do not support Python 3 which is currently the default Python version used with our GitHub Actions. The simplest way to workaround it properly seems to be using the Docker image that the maintainers of the project suggest to use for running the suite. This is more future-proof way that we should rely upon. This commit also removes some redundant steps that are not required.
6b59e70
to
12e3dca
Compare
Yeah, I also thought about that. I still enjoy the elegance and conciseness of Travis (compare it to GitHub Actions job definition that is much longer), but having both at the same time does not seem like a reasonable use of resources. Though we don't use Github Actions in |
See snapview/tokio-tungstenite#165 for more details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now, feel free to merge.
Use Autobahn Suite from Docker, symmetrical to snapview/tokio-tungstenite#165
See snapview/tokio-tungstenite#165 for more details.
See commit messages for more details.
NOTE: I also noticed that one of the files with results for Autobahn was [apparently blindly] copied in our repositories and is not used neither in
tungstenite-rs
nor intokio-tungstenite
. I tried to include this small clean up in this PR (could not hold myself leaving it as is), but I plan to submit a separate PR to do the same intungstenite-rs
. Additionally, there is a possibility to remove the code duplication for the tests that we do for Autobahn and to extract the common part, so thattokio-tungstenite
andtungstenite-rs
do not need to contain the same code.