smartdevicelink / sdl_evolution

Tracking and proposing changes to SDL's public APIs.
https://smartdevicelink.github.io/sdl_evolution/
BSD 3-Clause "New" or "Revised" License
33 stars 122 forks source link

[Accepted] SDL 0340 - ATF Selenium Support #1164

Closed theresalech closed 3 years ago

theresalech commented 3 years ago

Hello SDL community,

The review of of the revised proposal "SDL 0340 - ATF Selenium Support" begins now and runs through August 10, 2021. The original review of this proposal took place July 14 - July 27, 2021. The proposal is available here:

https://github.com/smartdevicelink/sdl_evolution/blob/master/proposals/0340-atf-selenium-support.md

Reviews are an important part of the SDL evolution process. All reviews should be sent to the associated Github issue at:

https://github.com/smartdevicelink/sdl_evolution/issues/1164

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of SDL. When writing your review, here are some questions you might want to answer in your review:

More information about the SDL evolution process is available at

https://github.com/smartdevicelink/sdl_evolution/blob/master/process.md

Thank you, Theresa Lech

Program Manager - Livio theresa@livio.io

dboltovskyi commented 3 years ago
  1. It might be noted that some of the negative test scenarios which are currently covered in ATF scripts looks like wouldn't be possible to rework within Selenium. E.g. scenarios where HMI doesn't respond to the requests from SDL.

  2. ATF already has a websocket client that connects to SDL Core but this piece will need to be extended to allow for validation of messages before they are passed to the HMI.

ATF already has such validation functionality. E.g. getHMIConnection():ExpectRequest() function allows to verify data which is sent by SDL to HMI.

  1. Besides SendResponse, SendRequest and SendNotification functions used to simulate sending messages from HMI, SendError function may also needs to be extended in order to send erroneous messages.

  2. On a diagram we suppose communication between SDL Core and Core Websocket Client should be bidirectional.

  3. Section RPC Switching defines Schema for RPC Switching. Does it mean this object needs to be defined in each test script? Could you provide more details how it is planned to be used? May be some example along with existing code? E.g. with:

    common.getHMIConnection():ExpectRequest("UI.Show", expectedParams)
Jack-Byrne commented 3 years ago
  1. Ok I can clarify this better in the Scope of Test Scripts section

  2. I can update the proposal to: "ATF already has a websocket client that connects to SDL Core, but this piece will need to be extended to pass messages to the HMI while using getHMIConnection():ExpectRequest() to validate RPC messages."

  3. I agree. A new section "Extending SendError" should be added to the proposal

  4. 👍

  5. I think there should be a default schema that all of the tests inherit, but individual tests can overwrite the object. This would be the same way that app configuration can be specified in a test but is not required. I will need some more time to work out a potential implementation.

theresalech commented 3 years ago

The Steering Committee voted to keep this proposal in review, to allow the author time to determine a potential implementation for Item 5, and for any other discussion to continue on the review issue.

Jack-Byrne commented 3 years ago

@dboltovskyi Here is an initial idea for how ExpectRequest would be extended in actions.lua

-- this would be contained in some config or common file
local HmiRpcSwitching = {
  ["UI.Show"] = {
    expectRequest = {
      validate = {
        {
          id = "#mainField1",
          type = "text",
          textParam = "mainField1"
        }
      }
    }
  }
}

function test.hmiConnection:ExpectRequest(pName, ...)
  local event = events.Event()
  event.matches = function(_, data) return data.method == pName end
  local args = table.pack(...)
  local ret = expectations.Expectation("HMI call " .. pName, self)
  if #args > 0 then
    ret:ValidIf(function(e, data)
        local arguments
        if e.occurences > #args then
          arguments = args[#args]
        else
          arguments = args[e.occurences]
        end
        reporter.AddMessage("EXPECT_HMICALL",
          { ["Id"] = data.id, ["name"] = tostring(pName),["Type"] = "EXPECTED_RESULT" }, arguments)
        reporter.AddMessage("EXPECT_HMICALL",
          { ["Id"] = data.id, ["name"] = tostring(pName),["Type"] = "AVAILABLE_RESULT" }, data.params)
        return compareValues(arguments, data.params, "params")
      end)
  end
  ret.event = event
  event_dispatcher:AddEvent(self, event, ret)
  test:AddExpectation(ret)
  if HmiRpcSwitching[pName] ~= nil and HmiRpcSwitching[pName].expectRequest ~= nil then
    ret.Do = function(_, func) 
      SeleniumManager.sendRequest(pname, args, HmiRpcSwitching[pName].expectRequest)
      return self 
    end
  end

  return ret
end

Behavior changes highlighted here:

  if HmiRpcSwitching[pName] ~= nil and HmiRpcSwitching[pName].expectRequest ~= nil then
    ret.Do = function(_, func) 
      SeleniumManager.sendRequest(pname, args, HmiRpcSwitching[pName].expectRequest)
      return self 
    end
  end

Idea is that if the RPC matches the expected request in the RPC Swtiching table, ATF will overwrite the Do method to prevent a normal test from responding for the HMI. The message would then be passed to the Selenium Manager. This manager would be responsible for forwarding the message to the HMI and performing the validate or action behavior.

dboltovskyi commented 3 years ago

@JackLivio

5. Thank you for the example. Now the idea is more clear.

theresalech commented 3 years ago

We believe we should be able to return this proposal for the following revisions, which have been agreed to by the author and commenter:

  1. Update "Scope of Tests Scripts" section to clarify which negative tests scenarios cannot work with Selenium.
  2. Change "ATF already has a websocket client that connects to SDL Core but this piece will need to be extended to allow for validation of messages before they are passed to the HMI." to ""ATF already has a websocket client that connects to SDL Core, but this piece will need to be extended to pass messages to the HMI while using getHMIConnection():ExpectRequest() to validate RPC messages." in "Core Websocket Client" section.
  3. Add "Extending SendError" section.
  4. Make communication between SDL Core and Core Websocket Client bidirectional in the Architecture Diagram.
  5. Update "Schema for RPC Switching" based on this comment from the author.
theresalech commented 3 years ago

The Steering Committee voted to return this proposal for the revisions described in this comment.

theresalech commented 3 years ago

The author has updated this proposal based on the Steering Committee's agreed upon revisions, and the revised proposal is now in review until August 10, 2021.

The specific items that were updated since the last review can be found in this merged pull request: #1169.

theresalech commented 3 years ago

The Steering Committee fully agreed to accept this proposal.

theresalech commented 3 years ago

Implementation Issues Entered: