luckyframework / lucky_flow

Automated browser tests for web applications. Similar to Ruby's Capybara.
https://luckyframework.github.io/lucky_flow/
MIT License
51 stars 8 forks source link

Issue with Flapping Tests #116

Open stephendolan opened 3 years ago

stephendolan commented 3 years ago

I'm currently leveraging LuckyFlow to test the entirety of https://luckycasts.com

I've never had an issue in my development environment, but in GitHub Actions I continue to get random tests that will fail with content like this:

Screen Shot 2021-01-20 at 9 25 08 AM

If I re-run the job, everything is fine and the test passes without any changes.

Here's that spec code:

  it "allows admin users to delete topics" do
    flow = TopicManagementFlow.new
    topic = TopicBox.create
    user = UserBox.create &.is_admin(true)

    flow.visit Admin::Topics::Edit.with(topic.id), as: user
    flow.click("@delete-topic-button")
    flow.accept_alert

    flow.should have_current_path(Admin::Topics::Index)
    flow.el("body", text: topic.label).should_not be_on_page
  end

Here's the relevant GitHub workflow definition parts:

name: Lucky App CI

on:
  push:
    branches:
      - main
  pull_request:

jobs:
  RunSpecs:
    runs-on: ubuntu-latest
    container:
      image: crystallang/crystal:0.35.1
    env:
      LUCKY_ENV: test
      DB_HOST: postgres

    services:
      postgres:
        image: postgres:12-alpine
        env:
          POSTGRES_PASSWORD: postgres
        ports:
          - 5432:5432
        # Set health checks to wait until postgres has started
        options: >-
          --health-cmd pg_isready
          --health-interval 10s
          --health-timeout 5s
          --health-retries 5

    steps:
      - uses: actions/checkout@v2

      - name: Install PostgreSQL client
        run: |
          apt-get update
          apt-get -yqq install libpq-dev postgresql-client

      - name: Install browser
        run: apt-get -yqq install chromium-browser

      - uses: actions/setup-node@v2.1.4
        with:
          node-version: "12.x"
      - name: "Install yarn"
        run: npm install -g yarn

      - name: Get yarn cache directory path
        id: yarn-cache-dir-path
        run: echo "::set-output name=dir::$(yarn cache dir)"

      - name: Set up Yarn cache
        uses: actions/cache@v2
        id: yarn-cache
        with:
          path: |
            ${{ steps.yarn-cache-dir-path.outputs.dir }}
            **/node_modules

          key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
          restore-keys: |
            ${{ runner.os }}-yarn-

      - name: Set up Crystal cache
        uses: actions/cache@v2
        id: crystal-cache
        with:
          path: |
            ~/.cache/crystal
            bin/ameba
            lib
          key: ${{ runner.os }}-crystal-${{ hashFiles('**/shard.lock') }}
          restore-keys: |
            ${{ runner.os }}-crystal-

      - name: Install shards
        if: steps.crystal-cache.outputs.cache-hit != 'true'
        run: shards check || shards install

      - name: Install yarn packages
        if: steps.yarn-cache.outputs.cache-hit != 'true'
        run: yarn install --frozen-lockfile --no-progress

      - name: Compiling assets
        run: yarn prod

      - name: Run tests
        run: crystal spec
matthewmcgarvey commented 3 years ago

Here's the relevant expectation class https://github.com/luckyframework/lucky_flow/blob/ff7c22d3c72365ededf7bc677669ed95264c2f37/src/lucky_flow/expectations/have_current_path_expectation.cr#L1

My only guess is that there's a delay in the current_path so that when it's checked it's different than when the failure message is made but that seems like it would need a fair amount of delay between the two

stephendolan commented 3 years ago

Thanks, @matthewmcgarvey! I'm going to try a few things to see if I can get this to happen locally. Until then I'd just be taking shots in the dark in a PR.

I at least wanted to make sure we had an issue in case folks came searching for the same problem, though :)

fbuys commented 2 years ago

I got the exact same error locally. My flow/spec had this:

visit Categories::Show.with(category_id: @category.id), as: @user
click "@delete-category"
accept_alert
should have_current_path(Categories::Index)

Add added a sleep to get it to work.

visit Categories::Show.with(category_id: @category.id), as: @user
click "@delete-category"
accept_alert
sleep 0.001
should have_current_path(Categories::Index)

Not sure if there might be a more elegant solution, but perhaps this helps, so we can add a fix to the framework? I think I might work with a shorter sleep too.