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

CASSGO-20 Fix integration tests #1833

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

Conversation

lukasz-antoniak
Copy link
Member

@lukasz-antoniak lukasz-antoniak commented Oct 17, 2024

  • Java 17 is installed for future testing.
  • C* 4.0.13 fails to start with CCM if only Java 17 is available.
  • Creating environment variables expected by CCM.

Copy link
Contributor

@joao-r-reis joao-r-reis left a comment

Choose a reason for hiding this comment

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

👍

Comment on lines 161 to 183
- name: Install Java
run: |
curl -s "https://get.sdkman.io" | bash
source "$HOME/.sdkman/bin/sdkman-init.sh"
echo "sdkman_auto_answer=true" >> ~/.sdkman/etc/config
# sdk list java

sdk install java 11.0.24-zulu
echo "JAVA11_HOME=$JAVA_HOME_11_X64" >> $GITHUB_ENV

sdk install java 17.0.12-zulu
echo "JAVA17_HOME=$JAVA_HOME_17_X64" >> $GITHUB_ENV

# by default use JDK 11
sdk default java 11.0.24-zulu
sdk use java 11.0.24-zulu
echo "JAVA_HOME=$JAVA_HOME_11_X64" >> $GITHUB_ENV
echo "PATH=$PATH" >> $GITHUB_ENV
- name: Install CCM
run: pip install "git+https://github.com/riptano/ccm.git@${CCM_VERSION}"
run: |
python3 -m venv ~/venv
~/venv/bin/pip install setuptools
~/venv/bin/pip install "git+https://github.com/riptano/ccm.git@${CCM_VERSION}"
Copy link

@ribaraka ribaraka Oct 22, 2024

Choose a reason for hiding this comment

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

How about an idea to create a separate script file to combine the duplicated installation steps for Java and CCM? You can then reuse this script across all jobs for setup. It will reduce duplication and centralize maintenance. This way, if you need to update the installation logic for Java or CCM in the future, you can do it in one place instead of multiple jobs. The workflow becomes more nice and readable with a simple call to the script.
Adding caching to the installation process can speed up builds by reusing dependencies (e.g., SDKMAN, Java versions, Python packages) across workflow runs.

I’ve put together an example of this approach, which you can check out here: ribaraka#26

Copy link
Contributor

Choose a reason for hiding this comment

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

I like that approach but just to clarify, the PR you linked is not a complete "replacement" to what we have because it's missing a few things like cassandra.yaml customization, the commands to run the tests, etc. It's meant more as an example of what could be incorporated to this current PR?

Choose a reason for hiding this comment

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

Yes, I quickly put together an example to clarify the idea.

@joao-r-reis
Copy link
Contributor

The GH action is failing justfyi

@lukasz-antoniak
Copy link
Member Author

lukasz-antoniak commented Oct 29, 2024

Yes, I was actively developing it. @ribaraka, I have refactored the code to GitHub action instead of shell script. This way I do not need to repeat caching steps.

pip install --upgrade pip setuptools

echo "Installing CCM..."
pip install "git+https://github.com/riptano/ccm.git@${CCM_VERSION}"

Choose a reason for hiding this comment

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

No newline at end of file.

Choose a reason for hiding this comment

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

However, there's no newline in the .github/workflows/main.yml file, so I assume it's not required.

@joao-r-reis joao-r-reis changed the title Fix integration tests CASSGO-20 Fix integration tests Oct 30, 2024
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.

3 participants