Long commit messages cause tests to fail on the main branch

[madwort]

Although tests passed in #3234 , after merging they reliably fail on the main branch with this error, which looks like some simple config thing

======================================================================
ERROR: setUpClass (frontend.tests.functional.test_charts.AnalyseSummaryTotalsTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/code/openprescribing/frontend/tests/functional/selenium_base.py", line 47, in setUpClass
    cls.browser = cls.get_browser()
  File "/code/openprescribing/frontend/tests/functional/selenium_base.py", line 61, in get_browser
    return cls.get_browserstack_browser()
  File "/code/openprescribing/frontend/tests/functional/selenium_base.py", line 136, in get_browserstack_browser
    return webdriver.Remote(
  File "/usr/local/lib/python3.8/site-packages/selenium/webdriver/remote/webdriver.py", line 157, in __init__
    self.start_session(capabilities, browser_profile)
  File "/usr/local/lib/python3.8/site-packages/selenium/webdriver/remote/webdriver.py", line 252, in start_session
    response = self.execute(Command.NEW_SESSION, parameters)
  File "/usr/local/lib/python3.8/site-packages/selenium/webdriver/remote/webdriver.py", line 321, in execute
    self.error_handler.check_response(response)
  File "/usr/local/lib/python3.8/site-packages/selenium/webdriver/remote/errorhandler.py", line 242, in check_response
    raise exception_class(message, screen, stacktrace)
selenium.common.exceptions.WebDriverException: Message: name cannot be more than 255 characters

[evansd]

That's weird. I can't see how that PR would trigger this error. Particularly not on this page which I don't think that PR affects at all. I wonder if it's a change in the remote browser thing we're using and this just happened to be the first PR merged after it came into effect.

[madwort]

Ah, it's because of the auto-generated BROWSERSTACK_BUILD_NAME, which includes the full text of the commit. In which case not worth worrying about unless somebody particularly needs a distraction? CI output extract:

  docker-compose run --service-ports test
  shell: /usr/bin/bash -e {0}
  env:
    BROWSERSTACK_USERNAME: ***-GitHubAction
    BROWSERSTACK_ACCESS_KEY: ***
    BROWSERSTACK_PROJECT_NAME: openprescribing
    BROWSERSTACK_BUILD_NAME: [main] Commit 9cdf2c2: Include PPU savings on presentations with no substitutes (#3234)

  Even where a generic presentation has no substitutable presentations it
  can still have wide variation in price-per-unit and we need to capture
  this.

  This increases the number of "substitution sets" (sets of equivalent
  presentations) over which we calculate savings from 4164 to 7749. Given
  that the calculation involves looping over these sets I expect this
  change to roughly double the time the calculation takes.

  The total savings figure for each organisation is calculated once and
  then cached so the only impact here will be felt just after we upload
  new data and the cache is being rebuilt. The extra delay might be mildly
  annoying but I think we can live with it.

  The PPU breakdown pages on the other hand will now just take twice as
  long to load. This isn't ideal, but I don't see a quick way round this
  given that the calculation currently misses an important class of
  savings.

  Closes #3233 [Workflow: 883]
    BROWSERSTACK_LOCAL_IDENTIFIER: GitHubAction-3395a8bb-c6f7-483e-96cf-b0cdfaae56fb

[evansd]

Ah, so looking at the docs it will pass on PRs because for PRs it sets BROWSERSTACK_BUILD_NAME as:

[<Branch-Name>] PR <PR-number>: <PR-title> [Workflow: <Workflow-number>]

But for the push event which runs after merge it uses:

[<Branch-Name>] Commit <commit-sha>: <commit-message> [Workflow: <Workflow-number>]

which ends up being too long.

I think we can fix it by changing this line to not contain the string BUILD_INFO: https://github.com/ebmdatalab/openprescribing/blob/9e1c80cd3dd1ec3c362316ca5f7e625dba9cd456/.github/workflows/main.yml#L46

Maybe we can just use the commit sha if that's accessible to us somehow at that point.

[madwort]

commit sha sounds good, I think we can use an env var - I'll make a PR

[madwort]

workaround didn't immediately work, hoping they'll fix it upstream https://github.com/browserstack/github-actions/issues/13#issuecomment-948649607

[richiecroker]

@madwort another tidying message - did this get fixed?