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

RUM-4079 chore: Migrate tools and smoke tests to GitLab #1921

Merged
merged 13 commits into from
Jun 25, 2024

Conversation

ncreated
Copy link
Member

@ncreated ncreated commented Jun 24, 2024

What and why?

πŸ“¦ 🧰 Following #1910, this PR migrates next portion of CI setup to GitLab:

  • βœ… Smoke tests:
    • whole SDK integration (iOS + tvOS) with Cocoapods, Carthage, XCFrameworks and SPM
    • SPM build for iOS, tvOS, macOS, macOS through Mac Catalyst and visionOS
  • βœ… Tooling tests
    • unit tests for http-server-mock, rum-models-generator, sr-snapshots and distribution package (python)

The following integrations are still running on Bitrise:

  • ❌ SR snapshot tests - these will be moved to GitLab after upgrading our macOS runner to Xcode 15.4 and installing the iOS 17.5 runtime.
  • ❌ Dogfooding automation
  • ❌ Release automation
  • ❌ E2E tests app upload to s8s

I will address the remaining items in subsequent PR(s).

How?

General remarks:

  • The new smoke-test stage uses macos:ventura runner (lower spec). This is to not consume parallelisation budget of macos:sonoma which can run test and test-ui for other branches while smoke tests are still performed on the current branch.
  • Xcode 15 is not activated by default in macos:ventura, so I added an option to runner-setup.sh to activate it. Ultimately, this will be done in the AMI (runner image).
  • I renamed dependency-manager-tests to SmokeTests for clarity and to better fit our conventions.
  • All smoke tests are ran via distinct Makefiles stored at SmokeTests/<dep manager>/Makefile. This is not different from what we had before. To better streamline these tests, each smoke Makefile now distinguishes make clean install test commands.
  • I also moved instrumented-tests/http-server-mock to tools/http-server-mock to better fit repo layout.

Makefile

Same as in #1910, smoke and tool tests are ran from the main Makefile. List of new key commands:

  • Tool Tests:
    • make tools-test - Runs tests for all repo tools.
  • Smoke Tests:
    • make smoke-test - Runs smoke tests in specified TEST_DIRECTORY (using OS, PLATFORM and DEVICE)
    • make smoke-test-ios-all - Runs all smoke tests adequate for iOS.
    • make smoke-test-tvos-all - Runs all smoke tests adequate for tvOS.
    • make spm-build - Builds Package.swift for specified SCHEME and DESTINATION.
    • make spm-build-ios - Builds Package.swift for iOS.
    • make spm-build-tvos - Builds Package.swift for tvOS.
    • make spm-build-visionos - Builds Package.swift for visionOS.
    • make spm-build-macos - Builds Package.swift for macOS and Mac Catalyst.

Ultimately, the whole workflow for integrating branches will look this:

Screenshot 2024-06-24 at 14 00 11

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference
  • Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

  • Run unit tests for Session Replay

@ncreated ncreated self-assigned this Jun 24, 2024
Comment on lines -299 to -312

# Helpers

ECHO_TITLE=./tools/utils/echo_color.sh --title
ECHO_ERROR=./tools/utils/echo_color.sh --err
ECHO_WARNING=./tools/utils/echo_color.sh --warn
ECHO_SUCCESS=./tools/utils/echo_color.sh --succ

define require_param
if [ -z "$${$(1)}" ]; then \
$(ECHO_ERROR) "Error:" "$(1) parameter is required but not provided."; \
exit 1; \
fi
endef
Copy link
Member Author

Choose a reason for hiding this comment

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

πŸ’‘ Moved to tools/utils/common.mk as now being shared with Makefiles used for smoke testing.

@ncreated ncreated marked this pull request as ready for review June 24, 2024 15:12
@ncreated ncreated requested review from a team as code owners June 24, 2024 15:12
Copy link
Contributor

@rtrieu rtrieu 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!

Copy link
Contributor

@ganeshnj ganeshnj left a comment

Choose a reason for hiding this comment

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

Looks great, a couple of questions nothing blocking.

fi
}

# Prints current git branch
Copy link
Contributor

Choose a reason for hiding this comment

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

/question

what happens when HEAD doesn't point to a branch?

Copy link
Member Author

Choose a reason for hiding this comment

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

Like in detached HEAD configuration? It will print just "HEAD" in such case. Although, this code isn't new - we've been using it for years in existing smoke tests. I don't think GitLab setup changes anything significant to break it 🀞.


# Prints current git tag (if any)
function current_git_tag() {
if [[ -n "$CI_COMMIT_TAG" ]]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

/question

are these env variable populated via gitlab?

Copy link
Member Author

Choose a reason for hiding this comment

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

Exactly πŸ‘

echo_succ "Already using Xcode version '$version'."
elif [[ -d "$XCODE_PATH" ]]; then
echo "Found Xcode at '$XCODE_PATH'."
if sudo xcode-select -s "$XCODE_PATH"; then
Copy link
Contributor

Choose a reason for hiding this comment

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

/question

Is the runner user already in elevated/sudo mode?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes it is πŸ‘Œ - tested, it works πŸ‘

@@ -13,5 +13,3 @@ A brief description of implementation details of this PR.

### Custom CI job configuration (optional)
- [ ] Run unit tests for Session Replay
- [ ] Run smoke tests
Copy link
Contributor

Choose a reason for hiding this comment

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

πŸ’ͺ

REPO_ROOT := ../../
include ../../tools/utils/common.mk

ifeq ($(PLATFORM), iOS Simulator)
Copy link
Contributor

Choose a reason for hiding this comment

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

/comment

IIRC, we updated SPM tests with macOS support, it not then we should include macOS and visionOS.

If you can do along with this PR :great else we can do as separate PT too.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is already there, in this PR πŸ‘. We distinguish:

  • smoke tests - integrate SDK and run basic UI test (launch the app β†’ check no crash)
  • spm build tests - build SDK for all supported platforms

The SmokeTest/spm/Makefile in question performs smoke testing. It integrates SDK into SPMProject via Xcode's SPM (using current branch), then launches it to assert there is no runtime crash. We do it for iOS and tvOS.

The spm.sh added earlier is renamed to spm-build.sh and ran from .gitlab-ci.yml:

make spm-build-ios
make spm-build-tvos
make spm-build-visionos
make spm-build-macos

It validates that Package.swift builds for all these platforms. Both iOS and tvOS builds might be considered redundant considering SmokeTest/spm/Makefile, but I wanted to avoid changing existing concepts.

@ncreated ncreated merged commit 85dcd0f into develop Jun 25, 2024
15 checks passed
@ncreated ncreated deleted the ncreated/RUM-4079/migrate-smoke-and-tool-tests branch June 25, 2024 13:33
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.

4 participants