Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: use virtual environment in functional tests #476

Open
wants to merge 64 commits into
base: trunk
Choose a base branch
from

Conversation

JeremyTubongbanua
Copy link
Member

@JeremyTubongbanua JeremyTubongbanua commented Dec 13, 2024

closes #211

- What I did

Functional tests

  • Modified functional tests CI to use virtual environment instead of production atSigns.
  • Added ATDIRECTORY_HOST and ATDIRECTORY_PORT options to the functional test's CMakeLists.txt
  • Modified functional test helper function signatures to no longer take string lengths; int functional_tests_set_up_atkeys(atclient_atkeys *atkeys, const char *atsign); and int functional_tests_pkam_auth(atclient *atclient, atclient_atkeys *atkeys, const char *atsign);

atclient

  • Implemented this new function void atclient_connection_set_read_timeout(atclient_connection *ctx, const uint32_t timeout_ms); in connection.c
  • Refactored atclient_set_read_timeout (in atclient.c) to use the new function mentioned above
  • atclient_tls_socket connect now uses portstr[6] instead of portstr[5], ensuring a null terminator

- How to verify it

  • See functional tests GHA

- Description for the changelog
ci: use virtual environment in functional tests

@JeremyTubongbanua JeremyTubongbanua self-assigned this Dec 13, 2024
@JeremyTubongbanua JeremyTubongbanua marked this pull request as ready for review December 13, 2024 21:01
@JeremyTubongbanua JeremyTubongbanua changed the title feat: venv tools ci: use virtual environment in functional tests Dec 13, 2024
Comment on lines 791 to 642
void atclient_connection_set_read_timeout(atclient_connection *ctx, const uint32_t timeout_ms) {
if (ctx == NULL) {
atlogger_log(TAG, ATLOGGER_LOGGING_LEVEL_ERROR, "ctx is NULL\n");
return;
}

mbedtls_ssl_conf_read_timeout(&(ctx->ssl_config), timeout_ms);
}

Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this method already exist? I recall editing this in the changes for #475

Copy link
Member

Choose a reason for hiding this comment

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

We only use a few sets of keys, can you remove the excess ones

Copy link
Member Author

Choose a reason for hiding this comment

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

8a526e2

Done here


CONTAINER_NAME="virtualenv"

sudo docker exec "$CONTAINER_NAME" supervisorctl start pkamLoad
Copy link
Member

Choose a reason for hiding this comment

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

don't put sudo in the script. Most systems should have configured docker so that it doesn't need super user permissions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in fb2e8a3

@XavierChanth
Copy link
Member

@JeremyTubongbanua see #481

@JeremyTubongbanua JeremyTubongbanua marked this pull request as ready for review December 25, 2024 03:28
Copy link
Member

@cpswan cpswan left a comment

Choose a reason for hiding this comment

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

A few comments that could be tidied later but otherwise LGTM

run: |
mkdir -p ~/.atsign/keys/
echo "${{ secrets.ATKEYS_12ALPACA }}" > ~/.atsign/keys/@12alpaca_key.atKeys
echo "${{ secrets.ATKEYS_12SNOWBOATING }}" > ~/.atsign/keys/@12snowboating_key.atKeys
ls -la
Copy link
Member

Choose a reason for hiding this comment

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

Looks like a stray debug line

- name: Add vip.ve.atsign.zone to /etc/hosts
run: |
echo "127.0.0.1 vip.ve.atsign.zone" | sudo tee -a /etc/hosts
cat /etc/hosts
Copy link
Member

Choose a reason for hiding this comment

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

More debug stuff

sudo apt-get update
sudo apt-get install -y docker-compose
sudo ./start_virtualenv.sh
sleep 5
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to test for the services being ready rather than sleeping?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use virtual environment for functional tests rather than production atsigns
4 participants