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

Introduce smoke tests in CI for instrumentation targets #3618

Closed
FareesHussain opened this issue Jul 30, 2021 · 2 comments · Fixed by #5335
Closed

Introduce smoke tests in CI for instrumentation targets #3618

FareesHussain opened this issue Jul 30, 2021 · 2 comments · Fixed by #5335
Assignees
Labels
enhancement End user-perceivable enhancements. Impact: Low Low perceived user impact (e.g. edge cases). Issue: Needs Break-down Indicates that an issue is too large and should be broken into smaller chunks. Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@FareesHussain
Copy link
Contributor

Add CI check to check if the targets will build successfully.

#3505 (comment) For details.

@FareesHussain
Copy link
Contributor Author

@BenHenning I'm wondering what rule to use here.

like you've mentioned #3505 (comment) here that we need something like

bazel test //instrumentation:build_instrumentation_targets

to check these.

To me it looks like a target that contains all the targets we need to build in the CI for these smoke tests.

but instead can't we just do bazel build //instrumentation:all here.
in this case we can also shift the oppia_test to the instrumentation package as in the 2.3 pr we need it to access a separate AndroidManifest.

@BenHenning BenHenning added this to the Beta MR2 milestone Jun 11, 2022
@Broppia Broppia added issue_type_infrastructure Impact: Low Low perceived user impact (e.g. edge cases). labels Jul 7, 2022
@BenHenning BenHenning added Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Issue: Needs Break-down Indicates that an issue is too large and should be broken into smaller chunks. Z-ibt Temporary label for Ben to keep track of issues he's triaged. issue_user_developer labels Sep 15, 2022
@BenHenning BenHenning removed this from the Beta MR2 milestone Sep 16, 2022
@seanlip seanlip added enhancement End user-perceivable enhancements. and removed issue_type_infrastructure labels Mar 28, 2023
@MohitGupta121 MohitGupta121 added the Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. label Jun 16, 2023
@BenHenning BenHenning self-assigned this Feb 7, 2024
@BenHenning
Copy link
Member

@BenHenning I'm wondering what rule to use here.

like you've mentioned #3505 (comment) here that we need something like

bazel test //instrumentation:build_instrumentation_targets

to check these.

To me it looks like a target that contains all the targets we need to build in the CI for these smoke tests.

but instead can't we just do bazel build //instrumentation:all here. in this case we can also shift the oppia_test to the instrumentation package as in the 2.3 pr we need it to access a separate AndroidManifest.

Didn't address this originally, but the main reason we can't use 'all' is by introducing a smoke test, we actually turn a build verification check into a test (i.e. one that's picked up by bazel test) which means it will be included in the possible output from ComputeAffectedTests. This is ideal since it ensures verification of these builds happens automatically in the current CI configuration and happens only when necessary (i.e. instrumentation dependencies change).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement End user-perceivable enhancements. Impact: Low Low perceived user impact (e.g. edge cases). Issue: Needs Break-down Indicates that an issue is too large and should be broken into smaller chunks. Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Development

Successfully merging a pull request may close this issue.

5 participants