UI Tests: prompt to run on all browsers when UI tests are changed

[Erin007]

By default our UI test suite on Drone will only run against Chrome. By adding a commit tag, you can run tests against other browsers. Occasionally, tests will be flaky in browsers other than Chrome, but we don't have insight into that flakiness until the changes hit the test machine. The flakiness then affects the deployment process and slows the team down to investigate.

To prevent introducing flakiness into the test machine, I set up a commit hook that will prompt to add a [test all browsers] tag to the commit message if feature files are included in the commit content. For example:

The [test all browsers] tag will ensure that the PR's Drone run includes testing against Chrome, Safari, Firefox, IE11, iPad and iPhone so we can better catch flakiness before it has a bigger impact on the team.

[uponthesun]

What are your thoughts on:

• commit hook vs. changing our circle.rake logic to detect these changes?
• should all browsers be used for all tests or just your changed file?

The commit message tag to run everything on all browsers makes sense to add, but I wonder if it's what we should use in the common case. Do the drone tests pass right now if you use that tag?

[Erin007]

What are your thoughts on:

• commit hook vs. changing our circle.rake logic to detect these changes?

I think it would be nice if a commit hook wasn't needed, and circle.rake just detected these files; For this pass I was trying leverage what already exists to mitigate the issue in a low-lift way. I can try that approach.

• should all browsers be used for all tests or just your changed file?

There's a time saving benefit to only running the changed file(s) on all browsers.

The commit message tag to run everything on all browsers makes sense to add, but I wonder if it's what we should use in the common case. Do the drone tests pass right now if you use that tag?

Drone tests do not pass when run in all browsers : ( Is there a fundamental difference between the test environment and Drone that means they aren't expected to pass on all browsers?

[uponthesun]

Drone tests do not pass when run in all browsers : ( Is there a fundamental difference between the test environment and Drone that means they aren't expected to pass on all browsers?

In theory no, but they use different code paths for setting up the environment, and there's also a lot of conditionals in the code base which check environment variables. It's not a good situation, but it's where we are unfortunately. Hopefully the approach to detect the changed feature files during the run isn't too bad to implement.

Created https://codedotorg.atlassian.net/browse/INF-327 to track the tech debt of the two environments being different.

[Erin007]

This issue bit me again today. Having this reminder would've sufficiently prevented derailing the deployment pipeline so I'm reviving the change in the hopes of getting it in rather than deluding myself into thinking I will have time to optimize a solution.

[Erin007]

code-dot-org/lib/rake/circle.rake

Line 72 in 9a7df40

RakeUtils.rake_stream_output 'test:changed'

Doesn't this mean that Drone will only run on the subset of changed tests unless you tell it otherwise?

[breville]

Should/Could we factor out the common CircleUtils.tagged?(TEST_ALL_BROWSERS_TAG)s into a variable?

[uponthesun]

code-dot-org/lib/rake/circle.rake

Line 72 in 9a7df40

RakeUtils.rake_stream_output 'test:changed'

Doesn't this mean that Drone will only run on the subset of changed tests unless you tell it otherwise?

Yes, for the unit tests. The UI tests entry point is here:

code-dot-org/lib/rake/circle.rake

Line 77 in 9a7df40

task :run_ui_tests do

[uponthesun]

Hey, sorry, didn't mean to block this the first time around. Thanks for improving the process!

[cforkish]

just to confirm that i understand how this would have helped you yesterday, the idea is that you wouldn't have added the [test all browsers] tag to that PR? were you aware it wasn't going to work on IE?

[Erin007]

just to confirm that i understand how this would have helped you yesterday, the idea is that you wouldn't have added the [test all browsers] tag to that PR? were you aware it wasn't going to work on IE?

I was NOT aware the it wasn't going to work on IE. I should have added the [test all browsers] flag, but I didn't (well, because it didn't exist) to catch the problem in the Drone run on my PR rather than waiting for the test machine to catch the issue and block the deploy pipeline. Basically I want a reminder to add the flag that will catch failures across browsers.

[cforkish]

ahh, i didn't catch that this is for drone, not for the test machine. makes much more sense now, and sounds super helpful! thanks for the clarification!