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

Modified vcenter_vm_scenario1 test target to run on the eco vsphere env #584

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

anna-savina
Copy link
Collaborator

Modified vcenter_vm_scenario1 test target to run on the eco vsphere env

@anna-savina anna-savina marked this pull request as draft February 24, 2025 09:52
Copy link

codecov bot commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.04%. Comparing base (a5f6118) to head (b46e137).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #584   +/-   ##
=======================================
  Coverage   37.04%   37.04%           
=======================================
  Files         145      145           
  Lines       11398    11398           
  Branches     2262     2262           
=======================================
  Hits         4222     4222           
  Misses       7176     7176           
Flag Coverage Δ
sanity 37.04% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

file: ../group_vars.yml
tags: eco-vcenter-ci

- name: Set facts for tests
Copy link
Collaborator

@mikemorency mikemorency Feb 24, 2025

Choose a reason for hiding this comment

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

I think we should add an assertion after this with a helpful error message if any of these are null. It would be helpful to show the test environment is somehow messed up, instead of having a dev start digging through the test/modules

@@ -4,7 +4,8 @@
vcenter_cluster_name: "Eco-Cluster"
vcenter_datacenter: "Eco-Datacenter"
vcenter_port: 443
rhel_9_3_iso_path: "[eco-nfs-datastore-iso] rhel-9.3-x86_64-dvd.iso"
rhel_9_3_iso_path: "[eco-nfs-datastore-iso] rhel-9.3-x86_64-boot.iso"
Copy link
Collaborator

@mikemorency mikemorency Feb 24, 2025

Choose a reason for hiding this comment

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

Tzach made a content library with these ISOs that might be better long term than the NFS datastore.
Theres also a content library dedicated to testing resources I set up for vmware.vmware, ansible_ci_resources or something like that. We could add these ISOs there

source ../init-eco.sh

# Generates a string starting with 'test-' followed by 4 random lowercase characters
TINY_PREFIX="test-$(uuidgen | tr -d '-' | cut -c1-4 | tr '[:upper:]' '[:lower:]')"
Copy link
Collaborator

@mikemorency mikemorency Feb 24, 2025

Choose a reason for hiding this comment

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

Suggested change
TINY_PREFIX="test-$(uuidgen | tr -d '-' | cut -c1-4 | tr '[:upper:]' '[:lower:]')"
TINY_PREFIX="test-vmware_rest-$(uuidgen | tr -d '-' | cut -c1-4 | tr '[:upper:]' '[:lower:]')"

Since we have 3 repos making test resources, we could differentiate the prefix so we know from where the resources are coming

Copy link
Collaborator

Choose a reason for hiding this comment

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

this file is referenced here: https://github.com/ansible-collections/vmware.vmware_rest/blob/main/development.md#optional-update-the-return-block-of-the-vmware-modules-using-the-test-suite
I think we can delete that whole section of the docs. It does more harm than good and now this file is being removed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll add to the new PR to clean up all outdated tests

@anna-savina anna-savina force-pushed the modify_vcenter_vm_scenario1_test branch from 88930ed to bc31d38 Compare February 26, 2025 08:04
@anna-savina anna-savina force-pushed the modify_vcenter_vm_scenario1_test branch from bc31d38 to 5e69946 Compare March 10, 2025 13:15
@anna-savina anna-savina force-pushed the modify_vcenter_vm_scenario1_test branch from 5e69946 to b46e137 Compare March 10, 2025 13:30
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.

2 participants