Skip to content

Conversation

@Zerpet
Copy link
Member

@Zerpet Zerpet commented Feb 25, 2025

Proposed Changes

This PR makes small tweaks to Management UI + Selenium workflows. The changes to the workflows
are mainly tidying up. There is an important change to run the full suite of Selenium tests
on main. Prior to this PR, on main we run the "short" suite. This was not intended. The
short suite is meant for PRs.

Types of Changes

  • Bug fix (non-breaking change which fixes issue #NNNN)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause an observable behavior change in existing systems)
  • Documentation improvements (corrections, new content, etc)
  • Cosmetic change (whitespace, formatting, etc)
  • Build system and/or CI

Checklist

Put an x in the boxes that apply.
You can also fill these out after creating the PR.
If you're unsure about any of them, don't hesitate to ask on the mailing list.
We're here to help!
This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING.md document
  • I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
  • I have added tests that prove my fix is effective or that my feature works
  • All tests pass locally with my changes
  • If relevant, I have added necessary documentation to https://github.com/rabbitmq/rabbitmq-website
  • If relevant, I have added this change to the first version(s) in release-notes that I expect to introduce it

Further Comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution
you did and what alternatives you considered, etc.

CI can configure this variable to use a dynamic variable e.g. `${{
worker.temp }}`
Those branches were for Bazel builds. Bazel was replaced in main and
4.0+
The workflow to tests PRs is meant to run the short suite for management
UI tests. On commits, we want to run the full suite to ensure that
management UI tests are passing.
@Zerpet Zerpet self-assigned this Feb 25, 2025
@Zerpet Zerpet marked this pull request as draft February 25, 2025 13:17
@Zerpet Zerpet marked this pull request as ready for review February 25, 2025 13:26
Copy link
Contributor

@MarcialRosales MarcialRosales left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

In certain environments, we may want to customise the docker image e.g.
to use a proxy to avoid docker hub rate limiting. The default behaviour
remains unchanged.

The `if` logic was broken because `uname -a` returns the entire uname,
including OS, Kernel version, machine type and what not. The string
always starts with the OS i.e. Linux or Darwin, therefore, the matching
for `arm*` was always false; therefore, it was always defaulting to the
`else` image, which happens to be multi-arch. However, it was using
`seleniarm`, which is a community driven effort, not the official
Selenium account.

In the official OSS image, version 123.0 is too old. The oldest
available is 127.0. This commit bumps to the latest available. We could
consider depending on version `4`. Version `4` refers to Selenium
version, whilst version 123.0/133.0 refer to the browser version.
@Zerpet Zerpet force-pushed the ci/update-selenium branch from 44ba23a to ef8b4fc Compare February 25, 2025 14:01
@Zerpet
Copy link
Member Author

Zerpet commented Feb 25, 2025

I made a last minute change via ef8b4fc, when I realised that the logic to select the selenium image was not correct. As a result, I had to bump the image version. Were we targeting Chromium 123.0 for a particular reason? E.g. as a minimum supported version.

I'm wondering whether using the latest available image version is ok.

@MarcialRosales
Copy link
Contributor

MarcialRosales commented Feb 25, 2025

I have tested your changes around the selenium/chromium docker image and it still works in both modes, silent and interactive. So you can go ahead. Thanks! @Zerpet

@Zerpet Zerpet merged commit 9f336b9 into main Feb 25, 2025
1 check passed
@Zerpet Zerpet deleted the ci/update-selenium branch February 25, 2025 16:31
Zerpet added a commit that referenced this pull request Feb 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants