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

Add worker assertions resource_processing_integration_test #15952

Conversation

Bencodes
Copy link
Contributor

Adding a few assertions to the worker and multiplex worker tests to ensure that the expected workers are in fact coming up.

@Bencodes
Copy link
Contributor Author

@ted-xie mind taking a look at this?

@sgowroji sgowroji added team-Android Issues for Android team awaiting-review PR is awaiting review from an assigned reviewer labels Jul 25, 2022
@ted-xie ted-xie self-assigned this Dec 6, 2022
@ted-xie
Copy link
Contributor

ted-xie commented Dec 6, 2022

LGTM, we'll import this.

@ted-xie ted-xie added awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally and removed awaiting-review PR is awaiting review from an assigned reviewer labels Dec 6, 2022
@sgowroji
Copy link
Member

sgowroji commented Dec 8, 2022

Hello @Bencodes, Could you please resolve the above conflict files. Above PR is awaiting to import. Thanks!

@Bencodes Bencodes force-pushed the add-asserts-to-test_persistent_resource_processor branch from e848087 to 16d47c3 Compare January 19, 2023 20:46
@Bencodes
Copy link
Contributor Author

@ted-xie I think something may have regressed here causing multiplex workers to no longer work, and these tests no longer pass against master even with all the Android related multiplex flags enabled.

@ted-xie
Copy link
Contributor

ted-xie commented Jan 24, 2023

@Bencodes Coincidentally, I was just looking at this yesterday and noticed the same thing. I'm working on a fix right now.

@Bencodes
Copy link
Contributor Author

@Bencodes Coincidentally, I was just looking at this yesterday and noticed the same thing. I'm working on a fix right now.

Sounds good. Let me know when you have that fix up and i'll rebase this branch against that to confirm.

copybara-service bot pushed a commit that referenced this pull request Jan 27, 2023
Unblocks merging for PR #15952.

RELNOTES:
PiperOrigin-RevId: 505149043
Change-Id: I1cb5ccb4c162f26fea51e0cdef8c8553f3a3d5df
@ted-xie
Copy link
Contributor

ted-xie commented Jan 27, 2023

@Bencodes I merged the fix for multiplexed workers. Could you rebase your PR branch? Once all the presubmit tests pass, we can try importing this again.

@ted-xie ted-xie added awaiting-user-response Awaiting a response from the author and removed awaiting-PR-merge PR has been approved by a reviewer and is ready to be merge internally labels Jan 27, 2023
@Bencodes Bencodes force-pushed the add-asserts-to-test_persistent_resource_processor branch from 63e0c00 to 32a1a21 Compare January 27, 2023 19:05
@Bencodes
Copy link
Contributor Author

@ted-xie rebased and updated the tests to use the correct flag. This should be ready to merge.

ted-xie added a commit to ted-xie/bazel that referenced this pull request Jan 27, 2023
Unblocks merging for PR bazelbuild#15952.

RELNOTES:
PiperOrigin-RevId: 505149043
Change-Id: I1cb5ccb4c162f26fea51e0cdef8c8553f3a3d5df
@sgowroji sgowroji removed the awaiting-user-response Awaiting a response from the author label Jan 30, 2023
@ted-xie
Copy link
Contributor

ted-xie commented Jan 30, 2023

@Bencodes This PR has been merged; thank you! I'll cherry-pick this change and bf94d9d into 6.1.0.

ted-xie added a commit to ted-xie/bazel that referenced this pull request Jan 31, 2023
Unblocks merging for PR bazelbuild#15952.

RELNOTES:
PiperOrigin-RevId: 505149043
Change-Id: I1cb5ccb4c162f26fea51e0cdef8c8553f3a3d5df
ted-xie pushed a commit to ted-xie/bazel that referenced this pull request Jan 31, 2023
Adding a few assertions to the worker and multiplex worker tests to ensure that the expected workers are in fact coming up.

Closes bazelbuild#15952.

PiperOrigin-RevId: 505690186
Change-Id: If732c9f040ec144395b5525a48d3a7d367fe244b
ted-xie added a commit to ted-xie/bazel that referenced this pull request Feb 7, 2023
Unblocks merging for PR bazelbuild#15952.

RELNOTES:
PiperOrigin-RevId: 505149043
Change-Id: I1cb5ccb4c162f26fea51e0cdef8c8553f3a3d5df
ted-xie pushed a commit to ted-xie/bazel that referenced this pull request Feb 7, 2023
Adding a few assertions to the worker and multiplex worker tests to ensure that the expected workers are in fact coming up.

Closes bazelbuild#15952.

PiperOrigin-RevId: 505690186
Change-Id: If732c9f040ec144395b5525a48d3a7d367fe244b
ShreeM01 added a commit that referenced this pull request Feb 7, 2023
* Fix multiplexed workers for Android busybox tools

Unblocks merging for PR #15952.

RELNOTES:
PiperOrigin-RevId: 505149043
Change-Id: I1cb5ccb4c162f26fea51e0cdef8c8553f3a3d5df

* Add worker assertions resource_processing_integration_test

Adding a few assertions to the worker and multiplex worker tests to ensure that the expected workers are in fact coming up.

Closes #15952.

PiperOrigin-RevId: 505690186
Change-Id: If732c9f040ec144395b5525a48d3a7d367fe244b

---------

Co-authored-by: Benjamin Lee <ben@ben.cm>
Co-authored-by: kshyanashree <109167932+kshyanashree@users.noreply.github.com>
hvadehra pushed a commit that referenced this pull request Feb 14, 2023
Unblocks merging for PR #15952.

RELNOTES:
PiperOrigin-RevId: 505149043
Change-Id: I1cb5ccb4c162f26fea51e0cdef8c8553f3a3d5df
hvadehra pushed a commit that referenced this pull request Feb 14, 2023
Adding a few assertions to the worker and multiplex worker tests to ensure that the expected workers are in fact coming up.

Closes #15952.

PiperOrigin-RevId: 505690186
Change-Id: If732c9f040ec144395b5525a48d3a7d367fe244b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Android Issues for Android team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants