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

Fix Autobahn Scripts #165

Merged
merged 3 commits into from Apr 23, 2021
Merged

Fix Autobahn Scripts #165

merged 3 commits into from Apr 23, 2021

Conversation

daniel-abramov
Copy link
Member

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 in tokio-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 in tungstenite-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 that tokio-tungstenite and tungstenite-rs do not need to contain the same code.

Copy link
Contributor

@strohel strohel left a 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.

scripts/autobahn-client.sh Outdated Show resolved Hide resolved
scripts/autobahn-server.sh Show resolved Hide resolved
scripts/autobahn-server.sh Outdated Show resolved Hide resolved
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.
@daniel-abramov
Copy link
Member Author

BTW, it seems that GH Actions work well - we can as well remove Travis, if it has been completely superseded by GH Actions.

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 tungstenite-rs, only in tokio-tungstenite, so I'll probably just remove Travis from here and leave if in tungstenite-rs (allows to enjoy and compare both).

Copy link
Contributor

@strohel strohel left a 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.

@daniel-abramov daniel-abramov merged commit 4686ae0 into master Apr 23, 2021
daniel-abramov added a commit to snapview/tungstenite-rs that referenced this pull request Apr 23, 2021
a-miyashita pushed a commit to givery-technology/tungstenite-rs that referenced this pull request Jul 20, 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.

None yet

2 participants