Opened on 03/18/2019 at 01:28:15 PM

Closed on 08/29/2019 at 04:05:36 PM

#7378 closed change (rejected)

Add logic to automation to skip test cases completely

Reported by: Ross Assignee:
Priority: P4 Milestone:
Module: Automation Keywords:
Cc: sebastian, kzar Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

Description

Background

The test automation currently loads and tests against each test page it encounters from the index list. This means that pushing test page(s) incompatible with the current implementation would cause the automation to report a fail when ran and cause CI to constantly fail (I believe?).

However, sometimes it is necessary to push test pages without also updating the automation to handle it. This could be because the automation change is time consuming or complex, or the page is required quickly for other parties, or the feature being tested is not yet available in builds etc.

Also, as a tester, I would like to have pages available for manual testing, rather than none at all. The automation can be updated and the flag removed later.

What to change

Change the automation logic to also skip test pages whose list entry has the class of skip.

Attachments (0)

Change History (5)

comment:1 Changed on 03/19/2019 at 12:43:20 AM by sebastian

We want to improve automated test coverage, and if we now make it optional for new test pages to be compatible with the test automation, our test automation becomes a second class target, and we likely will never catch up again.

So far what it took to maintain compatibility with the test automation didn't strike me as unreasonably time consuming or complex. But I would much appreciate if you would ask for help upfront rather than landing changes that are incompatible with the test automation.

comment:2 follow-up: Changed on 03/19/2019 at 10:48:32 AM by Ross

Thanks for the reply. Here are my thoughts:

  1. The test pages can be in 4 main states. (A) Test page exists and works with automation. (B) The test page exists but is skipped by automation. (C) The test page exists but breaks automation. (D) The test page does not exist.
  1. Obviously state (A) is the most ideal and is what should be strived for. But if not possible, we should go for (B). Both (C) and (D) are bad, but (D) is the worst because it doesn't even allow for manual testing. [Note: I realise I caused (C) yesterday].
  1. The testpages are not just an automation target (as far as I understand - Happy to be corrected). They are also for manual testing, for QA and developers, for other internal teams (like mobile), for external teams and partners, even the general public. Sometimes it will be necessary, or even just useful to put stuff up without having addressed the automation part yet.
  1. As a tester working on a complicated, wide reaching, time-consuming to test project, in no way do QA want to make automation a second class target. It's not compatiable with our current goals and does nothing to make QA any easier, just harder.
  1. We will need this flag eventually IMO. There will eventually be test cases that are not easily compatible with the current set up of the automation or would require a lot of change or work. The sitekey page is sort of an example of that.
  1. This flag would be used with the requirement of it being removed as quickly as possible.
  1. Running the automation is actually somewhat time consuming, as there is no way to just run the new test to see if it works (unless I'm missing something? Again, happy to be corrected). I suppose I could create a temporary index page with just that link, but I only just thought of that... so this point might be moot.
  1. This ticket wasn't created to be used with recently added generic hide/block pages, it was just created to prepare for the future, where if something needed to be added in response to stakeholders requests quickly, it wouldn't break automation like it did, it can just be done.
  1. You're right the automation changes for the recent test pages were not complex, but I was trying to be proactive in responding to other teams/external requests. I honestly probably could have waited a day for your thoughts first.
  1. In the end, my view is that I'd rather push 100 test pages right now, all marked skip and then work back through adding them to the automation compared to only adding them once the automation parts were done. Because then those things can be easily tested at least manually in the meantime, which is still better than not tested (or not tested easily).

comment:3 in reply to: ↑ 2 ; follow-up: Changed on 03/20/2019 at 12:29:05 AM by sebastian

Yes, in theory it would be better to have a test page that can only be tested manually than not having that test page at all. But if we still strive for improving automated test coverage of Adblock Plus, I think this approach will back fire.

If we lacked a test page so far, I fail to see how adding it right now (rather than a few days later) can suddenly be an urgency. Also with this line of argument, you could argue as well that we shouldn't review those changes at all, to get them out quicker, which is certainly not in line with how we work.

  1. We will need this flag eventually IMO. There will eventually be test cases that are not easily compatible with the current set up of the automation or would require a lot of change or work. The sitekey page is sort of an example of that.

We can figure it out when we get there. But with the cases we have seen so far I'm confident that we can manage without neglecting the test automation. Just point out on the review, if your changes aren't compatible with the test automation, and I will help you to make it work. Can we at least try this out first?

  1. Running the automation is actually somewhat time consuming, as there is no way to just run the new test to see if it works (unless I'm missing something? Again, happy to be corrected). I suppose I could create a temporary index page with just that link, but I only just thought of that... so this point might be moot.

Note that even with a way to skip tests for the automation, you still have to run the automation in order to make sure that it is skipped correctly. But as a workaround to only run the automation for a particular test during development, you could modify the code (either of the test pages to only list the test page you want to test, or of the automation in adblockpluschrome to skip all other test cases).

comment:4 in reply to: ↑ 3 Changed on 03/20/2019 at 11:19:50 AM by Ross

Replying to sebastian:

Yes, in theory it would be better to have a test page that can only be tested manually than not having that test page at all. But if we still strive for improving automated test coverage of Adblock Plus, I think this approach will back fire.

If we lacked a test page so far, I fail to see how adding it right now (rather than a few days later) can suddenly be an urgency. Also with this line of argument, you could argue as well that we shouldn't review those changes at all, to get them out quicker, which is certainly not in line with how we work.

I see your point. I'm attempting to play catch up to make testing easier/faster for releases but agree a few days every time is not an issue and would ensure proper reviews.


  1. We will need this flag eventually IMO. There will eventually be test cases that are not easily compatible with the current set up of the automation or would require a lot of change or work. The sitekey page is sort of an example of that.

We can figure it out when we get there. But with the cases we have seen so far I'm confident that we can manage without neglecting the test automation. Just point out on the review, if your changes aren't compatible with the test automation, and I will help you to make it work. Can we at least try this out first?

I agree we can figure it out once we get to those cases.

  1. Running the automation is actually somewhat time consuming, as there is no way to just run the new test to see if it works (unless I'm missing something? Again, happy to be corrected). I suppose I could create a temporary index page with just that link, but I only just thought of that... so this point might be moot.

Note that even with a way to skip tests for the automation, you still have to run the automation in order to make sure that it is skipped correctly. But as a workaround to only run the automation for a particular test during development, you could modify the code (either of the test pages to only list the test page you want to test, or of the automation in adblockpluschrome to skip all other test cases).

I will start doing this instead, I should have thought of it sooner.

Thanks for your thoughts!

comment:5 Changed on 08/29/2019 at 04:05:36 PM by sebastian

  • Resolution set to rejected
  • Status changed from new to closed

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from (none).
 
Note: See TracTickets for help on using tickets.