-
Notifications
You must be signed in to change notification settings - Fork 75
[PM-14425] [BEEEP] Add test plans #1106
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
base: main
Are you sure you want to change the base?
Conversation
…ck in BitwardenTestCase for skip simulator checking.
… unit test plans so they are empty on remote and only generated locally each time if needed.
Great job! No new security vulnerabilities introduced in this pull request |
Co-authored-by: Katherine Bertelsen <[email protected]>
Scripts/generate_test_plans.sh
Outdated
TEST_PLAN_SNAPSHOT_PATH="TestPlans/TestPlan-Snapshot.xctestplan" | ||
SNAPSHOT_TESTS_TARGET="BitwardenSharedTests" | ||
|
||
mint install atakankarsli/[email protected] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've decided not to include the installation of xctestplanner
in the Mintfile so it's only installed when actually being used. For now this script won't be called in CI/CD so there's no need to waste time installing xctestplanner
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cool!
A few questions from me:
- I'm seeing a few date/time failures in the tests. It looks like we previously set
TZ=UTC
as an environment variable. Could you add that to the test plans? - We used to have a splash screen that showed while testing. That relies on a
-testing
argument passed on launch, could you add that? - Should your comment for
git update-index --skip-worktree TestPlans/TestPlan-Snapshot.xctestplan TestPlans/TestPlan-Unit.xctestplan
be documented in the README?
Previous test settings:

It looks like the test plans don't include the Networking target, could you include that? I think that's why the test counts are different.

# Conflicts: # GlobalTestHelpers/Support/BitwardenTestCase.swift # project-pm.yml
… and view inspector tests. Added Testplan-ViewInspector test plan and configure it on PM.
…(files ending with SnapshotTests) and view inspector tests (files ending with ViewInspectorTests). Moved methods and clear files in the meantime. Snapshots are still disabled.
… new test file structure and suffixes: Unit (files ending with Tests), Snapshots (files ending with SnapshotTest) and View inspector (files ending with ViewInspectorTests).
…can configure that as an environment variable from the Test plans as needed.
@@ -0,0 +1,37 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💭 Thoughts on having all of these files under TestPlans
folder and just naming them accordingly?
E.g. TestPlans/TestPlan-ViewInspector-bwk.xctestplan
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was my initial thought as well but then I saw other folders structure we are using and followed the same pattern, like GlobalTestHelpers
, Configs
, Scripts
.
Nonetheless, I agree that it'd be better having the scheme specified per file instead of per folder in this case, but we would deviate from our current structure.
What do you think?
🎟️ Tracking
PM-14425
📔 Objective
Add test plans so we can separate Unit vs Snapshot vs View inspector tests vs All tests.
✂️ Tests split
This PR splits tests files (mostly
*ViewTests
) into three categories:*Tests
.InlineSnapshotTesting
so these are the exceptions why the InlineSnapshotTesting package product is still needed in such targets.*SnapshotTests
and that only includes snapshot tests.*ViewInspectorTests
and that only includes view inspector tests.📁 Folder structure
There are new test plans per scheme (Password Manager, Authenticator and Kit) following the same folder structure as in other places:
TestPlans
TestPlans-bwa
TestPlans-bwk
🧪 New test plans
And each folder have the same structure of test plans for each scheme following the patterns described above:
TestPlan-Default.xctestplan
: Default test plan including all tests for the scheme.TestPlan-Snapshot.xctestplan
: Test plan including only snapshot tests for the scheme.TestPlan-Unit.xctestplan
: Test plan including only unit (not snapshots nor view inspector) tests for the scheme.TestPlan-ViewInspector.xctestplan
: Test plan including only view inspector tests for the scheme.Tip
An environment variable
SKIP_SIMULATOR_CHECK_FOR_TESTS
has been added to be checked in the base test case so we can avoid checking the simulator name and version when we want. This is particular useful on Unit test plans where it's configured.⏰ Reminders before review
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes