Page MenuHomePhabricator

[services] Tests - Blob - Add Put
ClosedPublic

Authored by karol on Jun 15 2022, 3:10 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 20, 9:37 PM
Unknown Object (File)
Fri, Dec 20, 9:37 PM
Unknown Object (File)
Fri, Dec 20, 9:37 PM
Unknown Object (File)
Fri, Dec 20, 9:37 PM
Unknown Object (File)
Fri, Dec 20, 9:37 PM
Unknown Object (File)
Fri, Dec 20, 9:36 PM
Unknown Object (File)
Fri, Dec 20, 9:17 PM
Unknown Object (File)
Fri, Dec 20, 6:56 PM

Details

Summary

Depends on D4273

Adding Put method to blob's tests.

Test Plan

For now, this method's not invoked. The Rust project should build normally.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

karol edited the test plan for this revision. (Show Details)
karol added reviewers: tomek, varun.
tomek requested changes to this revision.Jun 20 2022, 5:54 AM
tomek added inline comments.
services/commtest/tests/blob/put.rs
46 ↗(On Diff #13517)

What if None was returned for some earlier request? We should detect it instead of silently returning Ok.

This revision now requires changes to proceed.Jun 20 2022, 5:54 AM
karol edited the summary of this revision. (Show Details)

address feedback - check if all states have been reached before returnig ok

add a condition for data exists if all states not reached

tomek requested changes to this revision.Jun 29 2022, 3:48 AM
tomek added inline comments.
services/commtest/tests/blob/put.rs
41 ↗(On Diff #13936)

Shouldn't we set it only during the last iteration?

52 ↗(On Diff #13936)

Why do we check if any of these is true? Shouldn't we check if both are true?

This revision now requires changes to proceed.Jun 29 2022, 3:48 AM
services/commtest/tests/blob/put.rs
41 ↗(On Diff #13936)

this somehow doesn't work even though it works with the inner scope, this is apparently a different case with all the generators and stuff. Rust is really silent about the generators, as Varun mentioned they are relatively new to the language.

41 ↗(On Diff #13936)

I'm going to revert this, we can create a follow-up about this, I couldn't hack it and I have no idea how can I make it work. @varun ?

The problem is I couldn't figure out how to modify anything inside of the generator's body:

let mut x = 0;
let outbound = async_stream::stream! {
  //...
  x += 1;
  //...
};
println!("=> {}", x);

The output would still be 0. I tried borrowing, using Arc (can Rc/Arc be mutated at all?), but nothing worked.
This has to be generator-specific stuff, but generators are not well documented afaik.

Something like this works like a charm:

let mut b = false;
{
  b = true;
};
println!("hey {}", b); // true

@varun

i'll try to play around with this tomorrow

In D4274#124268, @karol-bisztyga wrote:

The problem is I couldn't figure out how to modify anything inside of the generator's body:

let mut x = 0;
let outbound = async_stream::stream! {
  //...
  x += 1;
  //...
};
println!("=> {}", x);

The output would still be 0. I tried borrowing, using Arc (can Rc/Arc be mutated at all?), but nothing worked.
This has to be generator-specific stuff, but generators are not well documented afaik.

Something like this works like a charm:

let mut b = false;
{
  b = true;
};
println!("hey {}", b); // true

@varun

That's strange... one possible option might be to try Rc<RefCell<T>>, but there should be an easier way.

I'm going to revert this, we can create a follow-up about this, I couldn't hack it and I have no idea how can I make it work. @varun ?

Didn't have time to investigate the generator thing, and honestly not sure I'll get to it tomorrow either. Can you please create a follow-up task in Linear? You can assign it to me

This revision is now accepted and ready to land.Jun 30 2022, 2:58 PM
This revision was landed with ongoing or failed builds.Jul 8 2022, 2:30 AM
This revision was automatically updated to reflect the committed changes.