Un-brittle-ify backend E2E tests and have them run by default again in CI (#397)

* test running tests

* Add delay so that core knows about node before feed subscribes to limit chance of race

* move delaye to the right place

* Don't do expensive docker step untill we will be pushing the image

* docker test skipped as hoped, so push to 'true'

* just remove docker steps in github CI entirely since they aren't needed by anything (gitlab CI does this stuff now)

* run CI on pull requests too to catch PRs from forks

.github/workflows/backend.yml

@@ -6,6 +6,11 @@ on:
       - '.github/workflows/backend.yml'
       - 'backend/**'
       - '!frontend/**'
+  pull_request:
+    paths:
+      - '.github/workflows/backend.yml'
+      - 'backend/**'
+      - '!frontend/**'
 
 env:
   CARGO_TERM_COLOR: always
@@ -37,27 +42,6 @@ jobs:
       - name: Build
         run: cargo check --all-targets
 
-  docker:
-    name: Push tagged images to docker
-    runs-on: ubuntu-latest
-    steps:
-    - name: Checkout sources
-      uses: actions/checkout@v2.3.4
-
-    - name: Login to Dockerhub
-      uses: docker/login-action@v1
-      with:
-        username: ${{ secrets.DOCKER_USERNAME }}
-        password: ${{ secrets.DOCKER_PASSWORD }}
-
-    - name: Build and push template image for tagged commit
-      uses: docker/build-push-action@v2 # https://github.com/docker/build-push-action
-      with:
-        context: './backend'
-        push: ${{ startsWith(github.ref, 'refs/tags/') }}
-        tags: parity/substrate-telemetry-backend:latest
-        # add_git_labels: true
-
   fmt:
     name: Run rustfmt
     runs-on: ubuntu-latest
@@ -124,26 +108,3 @@ jobs:
 
       - name: Cargo test
         run: cargo test --verbose --jobs 1
-
-  e2e:
-    name: Run potentially brittle E2E tests
-    continue-on-error: true
-    runs-on: ubuntu-latest
-    steps:
-      - name: Checkout sources
-        uses: actions/checkout@v2.3.4
-
-      - name: Install Rust stable toolchain
-        uses: actions-rs/toolchain@v1.0.7
-        with:
-          profile: minimal
-          toolchain: stable
-          override: true
-
-      - name: Rust Cache
-        uses: Swatinem/rust-cache@v1.3.0
-        with:
-          working-directory: backend
-
-      - name: Cargo test
-        run: cargo test --verbose --jobs 1 e2e -- --ignored

.github/workflows/frontend.yml

@@ -9,6 +9,11 @@ on:
       - '.github/workflows/frontend.yml'
       - 'frontend/**'
       - '!backend/**'
+  pull_request:
+    paths:
+      - '.github/workflows/frontend.yml'
+      - 'frontend/**'
+      - '!backend/**'
 
 defaults:
   run:
@@ -41,18 +46,3 @@ jobs:
 
     - name: Build
       run: yarn build
-
-    - name: Login to Dockerhub
-      uses: docker/login-action@v1
-      with:
-        username: ${{ secrets.DOCKER_USERNAME }}
-        password: ${{ secrets.DOCKER_PASSWORD }}
-
-    - name: Build and Push template image
-      uses: docker/build-push-action@v2 # https://github.com/docker/build-push-action
-      if: matrix.node-version == '12.x'
-      with:
-        context: './frontend'
-        push: ${{ startsWith(github.ref, 'refs/tags/') }}
-        tags: parity/substrate-telemetry-frontend:latest
-        # add_git_labels: true

backend/telemetry_core/tests/e2e_tests.rs

@@ -53,7 +53,6 @@ fn ghash(id: u64) -> BlockHash {
 
 /// The simplest test we can run; the main benefit of this test (since we check similar)
 /// below) is just to give a feel for _how_ we can test basic feed related things.
-#[ignore]
 #[tokio::test]
 async fn e2e_feed_sent_version_on_connect() {
     let server = start_server_debug().await;
@@ -75,7 +74,6 @@ async fn e2e_feed_sent_version_on_connect() {
 
 /// Another very simple test: pings from feeds should be responded to by pongs
 /// with the same message content.
-#[ignore]
 #[tokio::test]
 async fn e2e_feed_ping_responded_to_with_pong() {
     let server = start_server_debug().await;
@@ -101,7 +99,6 @@ async fn e2e_feed_ping_responded_to_with_pong() {
 
 /// As a prelude to `lots_of_mute_messages_dont_cause_a_deadlock`, we can check that
 /// a lot of nodes can simultaneously subscribe and are all sent the expected response.
-#[ignore]
 #[tokio::test]
 async fn e2e_multiple_feeds_sent_version_on_connect() {
     let server = start_server_debug().await;
@@ -140,7 +137,6 @@ async fn e2e_multiple_feeds_sent_version_on_connect() {
 /// where the shard is waiting trying to send the next "add node" message, while the
 /// telemetry core is waiting trying to send up to the shard the next "mute node" message,
 /// resulting in a deadlock. This test gives confidence that we don't run into such a deadlock.
-#[ignore]
 #[tokio::test]
 async fn e2e_lots_of_mute_messages_dont_cause_a_deadlock() {
     let mut server = start_server_debug().await;
@@ -197,7 +193,6 @@ async fn e2e_lots_of_mute_messages_dont_cause_a_deadlock() {
 
 /// If a node is added, a connecting feed should be told about the new chain.
 /// If the node is removed, the feed should be told that the chain has gone.
-#[ignore]
 #[tokio::test]
 async fn e2e_feed_add_and_remove_node() {
     // Connect server and add shard
@@ -263,7 +258,6 @@ async fn e2e_feed_add_and_remove_node() {
 /// If nodes connect and the chain name changes, feeds will be told about this
 /// and will keep receiving messages about the renamed chain (despite subscribing
 /// to it by name).
-#[ignore]
 #[tokio::test]
 async fn e2e_feeds_told_about_chain_rename_and_stay_subscribed() {
     // Connect a node:
@@ -300,6 +294,10 @@ async fn e2e_feeds_told_about_chain_rename_and_stay_subscribed() {
         .send_json_text(node_init_msg(1, "Initial chain name", "Node 1"))
         .unwrap();
 
+    // Wait a little for this message to propagate to the core so that
+    // it knows what we're on about when we subscribe, below.
+    tokio::time::sleep(Duration::from_millis(500)).await;
+
     // Connect a feed and subscribe to the above chain:
     let (feed_tx, mut feed_rx) = server.get_core().connect_feed().await.unwrap();
     feed_tx
@@ -359,7 +357,6 @@ async fn e2e_feeds_told_about_chain_rename_and_stay_subscribed() {
 /// If we add a couple of shards and a node for each, all feeds should be
 /// told about both node chains. If one shard goes away, we should get a
 /// "removed chain" message only for the node connected to that shard.
-#[ignore]
 #[tokio::test]
 async fn e2e_feed_add_and_remove_shard() {
     let mut server = start_server_debug().await;
@@ -438,7 +435,6 @@ async fn e2e_feed_add_and_remove_shard() {
 
 /// feeds can subscribe to one chain at a time. They should get the relevant
 /// messages for that chain and no other.
-#[ignore]
 #[tokio::test]
 async fn e2e_feed_can_subscribe_and_unsubscribe_from_chain() {
     use FeedMessage::*;
@@ -553,7 +549,6 @@ async fn e2e_feed_can_subscribe_and_unsubscribe_from_chain() {
 }
 
 /// If a node sends more than some rolling average amount of data, it'll be booted.
-#[ignore]
 #[tokio::test]
 async fn e2e_node_banned_if_it_sends_too_much_data() {
     async fn try_send_data(max_bytes: usize, send_msgs: usize, bytes_per_msg: usize) -> bool {
@@ -605,7 +600,6 @@ async fn e2e_node_banned_if_it_sends_too_much_data() {
 }
 
 /// Feeds will be disconnected if they can't receive messages quickly enough.
-#[ignore]
 #[tokio::test]
 async fn e2e_slow_feeds_are_disconnected() {
     let mut server = start_server(
@@ -703,7 +697,6 @@ async fn e2e_slow_feeds_are_disconnected() {
 /// If something connects to the `/submit` endpoint, there is a limit to the number
 /// of different messags IDs it can send telemetry about, to prevent a malicious actor from
 /// spamming a load of message IDs and exhausting our memory.
-#[ignore]
 #[tokio::test]
 async fn e2e_max_nodes_per_connection_is_enforced() {
     let mut server = start_server(