This is a fine solution short-term until github implements the
yaml anchors support. The limitation of this method is that we
cannot re-use most of the steps again for production install test
builds.
Thanks, Anders for this solution.
Verifying everything is migrated correctly is a pain. This script
ensures everything is done correctly (previous commit message
contains explainations for the steps being ignored if; in case
of github-actions steps they are ignored because they are actions
specific):
"""
This script prints out the ignore steps first. Then
prints out each step of both circle and actions side-by-side.
One step is out of order for bionic but verfying correction is
still easier. Format:
Actions: Install dependencies
Circle CI: install dependencies
....
"""
import yaml
with open('.circleci/config.yml') as f:
circleci_config = yaml.safe_load(f)
with open('.github/workflows/zulip-ci.yml') as f:
actions_config = yaml.safe_load(f)
circle_bionic_steps = []
circle_focal_steps = []
actions_bionic_steps = []
actions_focal_steps = []
"""
We ignore casper artifact upload, save_cache, and
store_tests_reports steps.
"""
def get_circleci_steps(job, arr):
for step in circleci_config['jobs'][job]['steps']:
if isinstance(step, str):
arr.append(step)
continue
step_name = step.get('run', {}).get('name', False)
if not step_name:
if step.get('restore_cache'):
key = step['restore_cache']['keys'][0].split('.')[0]
step_name = f'<restore-cache> {key}'
elif step.get('store_artifacts', False):
destination = step['store_artifacts']['destination']
step_name = f'<store-artificats> {destination}'
if destination == 'casper':
\# This is no longer needed
print('Ignoring step:')
print(step)
print()
continue
else:
"""
We don't care about save_cache; github-actions
does this automatically, and store_tests_reports
is circelci timing specific.
"""
print('Ignoring step:')
print(step)
print()
continue
if step_name != 'On fail':
arr.append(step_name)
get_circleci_steps('bionic-backend-frontend', circle_bionic_steps)
get_circleci_steps('focal-backend', circle_focal_steps)
""" We ignore there steps specific to github-actions"""
for step in actions_config['jobs']['focal_bionic']['steps']:
BOTH_OS = 'BOTH_OS'
if_check = step.get('if', BOTH_OS)
step_name = step.get('name')
if step_name is None:
step_name = step['uses']
if (
step_name == 'Upgrade git for bionic' or
step_name == 'Add required permissions' or
step_name == 'Move test reports to var'
):
print('Ignoring step:')
print(step)
print()
"""These are github-actions specific; see comments"""
continue
if if_check == BOTH_OS:
actions_bionic_steps.append(step_name)
actions_focal_steps.append(step_name)
elif 'is_bionic' in if_check:
actions_bionic_steps.append(step_name)
else:
actions_focal_steps.append(step_name)
bionic = zip(circle_bionic_steps, actions_bionic_steps)
focal = zip(circle_focal_steps, actions_focal_steps)
print('Bionic steps:')
for (circle_step, actions_step) in bionic:
print(f'CircleCI: {circle_step}')
print(f'Actions: {actions_step}')
print()
print('Focal steps:')
for (circle_step, actions_step) in focal:
print(f'CircleCI: {circle_step}')
print(f'Actions: {actions_step}')
print()
Doing service memcached start instead of restart fixed an issue on
focal build in GitHub actions, where it exits with code 1 when it
is done twice.It is done first in Install Dependencies step and then
again in last step where we call tools/ci/setup-backend again which
runs provision.
Furthermore, I don't belive there is a technical reason we use
restart over start; rather I think it was just a random choice with
the intend to just start the services in CI. I traced the code back
to commit 1f2f497cab if it helps.
Looking at the source code of memcached, the step that's failing is:
start-stop-daemon --stop --quiet --oknodo --pidfile $PIDFILE
which is equivilent to: service memcached stop, we can rule out the
service memcache start since it works. Ideally, we do figure out and
solve the issue of why memcached fails when executing service
memcached stop but I am not equipped with debugging it. And this
workaround seems reasonable rather than a "hacky" solution.
For the relevant code in memcached see:
https://github.com/memcached/memcached/blob/master/scripts/memcached-init.
Finally, the change to the rest of services is for consistency.
Some noteable diffrence from circleci:
- We upgrade git to newer version (reason explained in comments)
- We set HOME to /home/github (also explained in comments)
- Adjust permissions (... comments)
- Minor changes to step names and cache keys.
- We don't need to port the save_cache steps they are done
automatically in actions. And, we did not port the
store_test_results step which is circleci specific.
- We didn't port the notify_failure step yet (see the TODO).
Log RealmAuditLog in do_set_realm_property and do_remove_realm_domain.
Tests for the changes are written in test_events because it will save
duplicate code for test_change_realm_property.
Added new Event Type in AbstractRealmAuditLog STREAM_CREATED.
Since we finally create streams in create_stream_if_needed function
in zerver/lib/streams.py so logged realm_audit there.
Passed acting_user when create_stream_if_needed or ensure_stream
function is called.
Added tests in test_audit_log.
This commit moves out the SoftDeactivationMessageTest out of
test_messages.py (which at the moment have mixed category of tests) into
a more logical file, test_soft_deactivation.py.
This commit moves TestMessageForIdsDisplayRecipientFetching class which
have tests regarding display_recipient filled in by MessageDict to
test_message_dict.py.
This commit moves InternalPrepTest test class to test_message_send.py
because it tests internal_send_* and internal_prep_* functions which are
used for internal message sending in zulip.
This commit moves few tests related to testing proper sending of private
messages from PrivateMessagesTest class in test_messages.py to a new class
in test_message_send.py.
The commit moves, test_create_mirror_user_despite_race which is not related
to message sending from MessagePOSTTest class in test_message_send.py to
test_mirror_users.py.
This commit extracts out MessagePOSTTest class from test_messages.py
intially.
In future commits other related message sending tests will be moved from
test_messages.py to test_message_send.py.
Starting with extracting out MirroredMessageUsersTests as it is related to
mirror users than anything message-specific.
In a future commit, may extract out some tests from MessagePOSTTest as well
but still deciding on those.
To make the typeahead code more readable, we extract this function to
timerender. We also improve the logic to be more readable, and add tests
to confirm its validity.
We have moved our invalid timestamp logic to use timestamp-error class,
however, if there are any valid outputs by the backend markdown that
the frontend considers invalid, we want to debug them. This commit
adds tooling to ensure we log those error messages.
We had been using !time() syntax for timestamps so far. Since its
an unreleased feature, we can make changes without affecting many
people.
Fixes#15442.
This commit fixes a bug where clicking on a stream row on the left
in the subscriptions table called the ".stream-row" as well as the
".sub_unsub_button" click events in `stream_edit.js`.
This caused the stream subscription to toggle everytime the row was
clicked. Also, this bug is only observed if the ".sub_unsub_button"
had been clicked first.
The previous element passed was a simplebar container which calculated
the incorrect height, thus `scroll_util.scroll_element_into_container`
had no effect.
There is a bug and race issue that occurs when a message is selected
while we are in the process of reifying a locally echoed message,
raising the "Selected message id not in MessageList" error.
The code flow to get the exception is as follows:
* A user sends a message to the current narrow we are in.
* Before the new message event is received, we sent a message to
the same message list which renders it with a locally echoed id.
* One of the ways of getting the exception is to already have the
locally sent message selected, before receiving an acknowledgment
from the server.
* Thus the Message List Data's `selected_id` now points to the new
message id. The exception is raised on entering the `was_selected`
if block inside `message_list_view` which tries to re-select the
message.
Updating the `_rerender_message` code for this special case won't fix
the entire bug because, as mentioned above there are other ways of
getting the exception:
Ideally, after all our synchronous work (`echo.process_from_server`)
has completed we would expect the re-order and re-render work of the
`change_message_id` would occur first, due to the timer of the
setTimeout being set to 0.
However as evident from the race condition existing, this isn't always
the case. `change_message_id` function is responsible for 3 things:
updation, re-ordering and re-rendering.
The first one which is responsible for updating the message list's
local cache, occurs synchronously while for the latter two, they both
occur asynchronously.
Before the setTimeout which is responsible for the latter two actions,
is encountered the user might select the message by clicking or more
commonly by scrolling, which causes this message selection event to be
ahead of the setTimeout in the callback queue.
During this time frame, our race condition takes place.
And even though the message id is updated it's Message List is not
in the correct sort order, which leads to `closest_id` !== `id` in
`MessageList_select_id` being true and raising the exception.
Now, we only asynchronously call the re_render function, to guarantee
the data is always correct and UI updates should be done at the end.
Extended by tabbott to comment the setTimeout call.
Fixes#15346.
Alex Vandiver
Priyank Patel
arpit551
Mohit Gupta
Anders Kaseorg
Aman Agrawal
Rohitt Vashishtha
Aman Agrawal
Ryan Rehman