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.

Sign up for GitHub

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

Jump to bottom

Non-modularized testing module tests not being built in Bazel today #3703

Closed
BenHenning opened this issue Aug 18, 2021 · 3 comments · Fixed by #3659
Closed

Non-modularized testing module tests not being built in Bazel today #3703

BenHenning opened this issue Aug 18, 2021 · 3 comments · Fixed by #3659
Labels
Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@BenHenning
Copy link
Member

Found this out in passing.

bazel query 'kind(test,//testing/...)' only returns the tests that have been migrated rather than all. From some digging this seems to be because having a BUILD.bazel file (even a blank one) under src/test changes the directory graph such that the glob can no longer find files under that directory. This is actually really unexpected, and may pose an issue as we continue to modularize tests.
@BenHenning
Copy link
Member Author

/cc @aggarwalpulkit596 as FYI

@BenHenning
Copy link
Member Author

Okay, this does seem by design: https://docs.bazel.build/versions/main/be/functions.html#glob. Glob will not match files within directories if those files are a subpackage (which is being established by exporting the AndroidManifest file). I'm going to see if we can move that export up a level and avoid accidentally creating a subpackage.

@BenHenning
Copy link
Member Author

Moving the test manifest filegroup to the testing/ package and removing the testing/src/test subpackage seemed to do the trick. Not so worried about this happening again so long as we following //testing module's pattern for modularizing tests moving forward.

@BenHenning BenHenning mentioned this issue Aug 18, 2021
Merged
8 tasks
BenHenning added a commit that referenced this issue Aug 19, 2021
@MaskedCarrot @yashraj-01 @BenHenning
Fix #2476, #3312 and #3703: Move hint handler to domain layer (#3659)
d2372ae 
* moved hint handler to domain layer

* fixed app layer espresso tests

* fix app layer robolectric tests

* fixed domain layer tests

* Added annotations to test exemptions

* proto lint fix

* fixed hint handler for training sessions

* nit and removed test excemptions

* added hint tests for config change

* fixed test file exemptions

* fixed failing test

* made HintHandler injectable

* fixed ktlint error

* Added tests for hint handler

* nits

* fixed failing build

* fixed failing build

* fix build

* fixed imports

* nits and improved testing

* updated exploration.proto

* removed progress controller from kdoc exemptions

* fix failing test

* moved timer to domain

* fixed build and nits

* nit

* added listener back to test exemptions

* nit fixes and added more tests

* lint fix

* First round: make HintHandler independent.

This also brings HintHandler into an interface + factory pattern. This
isn't the final design since I think we can largely simplify the way
hints work; that'll be my next pass.

This breaks questions & HintHandlerTest; those will require further work
later.

* Simplify HelpIndex in proto.

Move HelpIndex to PendingState to avoid the entire domain case of
handling CompletedState.

* Simplify hints & solutions.

This removes the per-Hint/Solution tracking & completely leans on
HelpIndex for proper hints & solution tracking both in the domain & UI
layers.

Fixes a bunch of other stuff, too, including
QuestionAssessmentProgressController tests.

* Clean up dead code paths & improve handler API.

* fixed test modules and lint

* renamed HintHandlerTest to HintHandlerImplTest

* Add tests for HintHandler.

This introduces some new explorations for making testing HintHandlerImpl
easier.

* Add remaining tests/exemptions for new files.

* Lint fixes.

* Post-merge fixes (including lint fixes).

* Post-merge maven_install fix.

* Revert "Merge branch 'develop' into move-hint-handler-to-domain"

This reverts commit e2fea90, reversing
changes made to 6659858.

* Post-merge Gradle-discovered fixes.

* Revert "Revert "Merge branch 'develop' into move-hint-handler-to-domain""

This reverts commit b1622c0.

* Additional post-merge fixes.

* Address first batch of review comments.

(Clarified some proto fields & remove trailing comma).

* Fix testing module tests & remove unnecessary changes.

* Simplify & fix more reviewer comments.

This simplifies some data pipelining in the UI, and improves HintHandler
documentation in addition to fixing some more reviewer comments.

* Improve documentation to address review comment.

* Rename new module to prod module.

This simplifies the changes & approvals needed in #3705.

* Rename proto field (to address review comment).

Also, fix broken reference error accidentally introduced in an earlier
commit.

* Kotlin lint fixes.

Co-authored-by: yashraj-01 <[email protected]>
Co-authored-by: Ben Henning <[email protected]>
@BenHenning BenHenning added the Z-ibt Temporary label for Ben to keep track of issues he's triaged. label Sep 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
[Task Force] Migrate to Bazel - Android
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #2476, #3312 and #3703: Move hint handler to domain layer oppia/oppia-android
1 participant
@BenHenning