weixin_39662834的博客

hi , finally found some free time to finalize the PR ☺️. ready for the review 🤞

Acceptance test results:

$ make testacc TEST=./aws TESTARGS='-run=TestAccAWSCodeStarNotificationsNotificationRule_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSCodeStarNotificationsNotificationRule_ -timeout 120m
=== RUN   TestAccAWSCodeStarNotificationsNotificationRule_Basic
=== PAUSE TestAccAWSCodeStarNotificationsNotificationRule_Basic
=== RUN   TestAccAWSCodeStarNotificationsNotificationRule_Status
=== PAUSE TestAccAWSCodeStarNotificationsNotificationRule_Status
=== RUN   TestAccAWSCodeStarNotificationsNotificationRule_Targets
=== PAUSE TestAccAWSCodeStarNotificationsNotificationRule_Targets
=== RUN   TestAccAWSCodeStarNotificationsNotificationRule_Tags
=== PAUSE TestAccAWSCodeStarNotificationsNotificationRule_Tags
=== RUN   TestAccAWSCodeStarNotificationsNotificationRule_EventTypeIds
=== PAUSE TestAccAWSCodeStarNotificationsNotificationRule_EventTypeIds
=== CONT  TestAccAWSCodeStarNotificationsNotificationRule_Basic
=== CONT  TestAccAWSCodeStarNotificationsNotificationRule_Tags
=== CONT  TestAccAWSCodeStarNotificationsNotificationRule_Targets
=== CONT  TestAccAWSCodeStarNotificationsNotificationRule_Status
=== CONT  TestAccAWSCodeStarNotificationsNotificationRule_EventTypeIds
--- PASS: TestAccAWSCodeStarNotificationsNotificationRule_Basic (23.47s)
--- PASS: TestAccAWSCodeStarNotificationsNotificationRule_Targets (50.69s)
--- PASS: TestAccAWSCodeStarNotificationsNotificationRule_Status (50.80s)
--- PASS: TestAccAWSCodeStarNotificationsNotificationRule_Tags (51.72s)
--- PASS: TestAccAWSCodeStarNotificationsNotificationRule_EventTypeIds (58.92s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws   61.444s

hi , I am not sure what is the current community contribution review process, but any chance this can be reviewed soon? or am i missing something for proper PR?

It is not clear to me why the introduction of a new attribute was needed here. The only issue with #2800 was that the "StateFunc" for "user_data" did double encoding of already Base64 encoded data.

To address this issue we could have updated use "StateFunc" for "user_data" to utilize the "userDataHashSum" function.

Except for the one issue from #2800 everything up until this pull request handled both base64 and non-base64 encoded data with one attribute.

The issue in #4197 is fixed by the PR without even using the new attribute as we actually make the StateFunc change here.

I want something closer to "the whole truth". In typescript, all conforming to a shape { _tag: 'Left', left: E } means is that a value has those properties, with those types.

For "the whole truth" we would need to define all possible attributes that are not allowed to be on the Left or Right value which in the end seems to be an infinite set of properties. So by setting one attribute that is not allowed you get no where nearer to the "truth".

Besides that you make the DX worse cause now right? and left? are prompting up when you try to define your types manually.

Having to import helper functions seems like unnecessary overhead, which will inevitably lead to developers taking dangerous shortcuts.

Taking shortcuts in programming leads always to bad code quality but it does not harm the correctness of your program even when the developer adds attributes that do not belong to this type.

Yes, it is an inconsistency. I have fixed this inconsistency on the branch in PR #2166. Unfortunately this PR has stalled for a while because the last couple of issues have proven more intractable than I had anticipated, and because of time pressures on my side. Hopefully soon...

, if this works for you then go for it, but there are a couple of drawbacks to that approach. (It also strikes me as a bit of a "code smell" when we have to add extra layers of encapsulation for something as simple as calling a single method in the System class for no other reason than to make it testable.) Especially if you're working on an existing body of code, this will mean going back and retrofitting the TimeProvider interface to all the existing code, and if you miss one or two you might end up with inconsistent results. On a larger project with multiple developers you always run the risk of people forgetting to use the encapsulated interface and instead calling System.currentTimeMillis() directly.

Also, the TimeProvider interface approach will only work for simple unit tests. If you are trying to integrate your tests with third party libraries that rely on the current time (more complicated unit tests or integration tests), the TimeProvider interface approach will produce inconsistent results because they won't access the system time through the mocked interface.

The Robolectric approach doesn't suffer from either of these weaknesses.- it will work in both cases (provided you configure Robolectric to instrument the third party libraries also).

I'm not trying to tell you how to do it, but I thought to let you know of the options.

Hi , ShadowSystemClock.currentTimeMillis() doesn't have an ` annotation because it is not actually implementing a method in theSystemClock` class like a shadow normally does. As noted in an earlier post in this thread, it is invoked via an interception handler, which redirects the method call on the caller's side rather than on the callee's side.

As I noted in my answer on StackOverflow, there is an instrumentedPackages option (which may not have been available when first raised this issue) in the ` annotation to specify additional packages to instrument. If you specify the package(s) that you are testing to this annotation, then they should get instrumented (disclaimer: I haven't tried it myself recently) - which means that when they callSystem.currentTimeMillis()they will retrieve the time fromShadowSystemClock.currentTimeMillis()` instead of the real current system time.

Hope that this helps; let us know if you get stuck again.

Just curious , but were you running this code on the UI thread or another thread/async task? I don't think it would be good to use this on the UI thread because it would block the UI thread, which is a no-no if you want to maximise the app responsiveness. You'd be better off posting poll events to the UI thread and checking the results in the poll handler.

The reason I ask is because in the fixes I'm looking at for #1879 and the architecture I'm trying to put together, I can probably make this code work as-is without the need to shadow it, but it would only work (at least, only work easily) if called on a non-UI thread. This is a consequence of the fact that the test thread shares the UI thread. I'm thinking to actually make Robolectric assert if wait() or sleep() is called from the UI thread - not only will this greatly simplify the implementation, but it will also help developers find bugs in their code. The only other alternative would be to separate the UI thread from the test thread, but this would complicate the test implementation.

thanks for clarification.

Is there any reference how haskell/purescript would implement something like this?

why not extend the api with a Tag Ord something like

ts
const ordU = getUnionOrd('type', ordString, {
  a: contramap((x: A) => x.a, ordString),
  b: contramap((x: B) => x.b, ordNumber)
})

This would allow the same as .

What I dislike is the definition of 'type'. Why can we not extract this from a definition of the resulting ord type? Something like

ts
const ordU = getUnionOrd(contramap((x) => x.type, ordString), {
  1: contramap((x: A2) => x.a, ordString),
  2: contramap((x: B2) => x.b, ordNumber)
});

But then you have lookup problem of the key inside the function, which could be simulated with the ordString.equals over all keys of the bodies parameter.

Please add this to the examples section. The contramap seems to be a really common function to defined Ord on objects.

missed some code. The as is code is correct

Currying for the rescue?

ts
pipe(
  chain(x => bar),
  map(baz)
)(formNullable(foo))

Or maybe we could create something like

ts
flow(option).chain(x => bar).map(baz).exec(fromNullable(foo))

// or

flow(option)(fromNullable(foo)).chain(x => bar).map(baz)

Tying everything to the master scheduler actually has the opposite effect on code execution order - by having all looper threads and the async task executors synchronised to the same master scheduler, their tasks will always run in the same repeatable order. Without a single master scheduler, the order in which they run depends on the order in which you advance each of the individual schedulers, and if you have individual schedulers then each thread ends up with a view of the system time that might not be consistent with the other threads.

On the other hand, the first point you raised is true - there are certain types of multithreaded code that would deadlock if tested with a single master scheduler. If, for example, you had two runnables on two threads (A & B), and thread A's runnable was supposed to wait for thread B's, but thread B's runnable which issued the notify() (or equivalent) was not scheduled to run until after thread A's, then it would deadlock.

I didn't consider this a major problem because: 1. such code is rare in most Android apps, 2. such apps can't be tested on Robolectric as they currently stand anyway (so my solution would at least have been an incremental improvement), 3. I made it configurable for backwards compatibility, so if you need individual control over the schedulers to avoid deadlock you can get it, and 4. such problems are generic to testing multithreaded code in a repeatable way anyway - they are not specific to Robolectric.

Sorry, no progress. I haven't touched Android for a year.

I think that my solution would have solved most of what you have quite rightly called "ugly".

No, sorry. On 09/03/2016 5:40 AM, "Constantine Voronin" notifications.com wrote:

This method (RuntimeEnvironment.getMasterScheduler()) is not included in Release 3.0 https://github.com/robolectric/robolectric/releases/tag/robolectric-3.0, is it?

— Reply to this email directly or view it on GitHub https://github.com/robolectric/robolectric/issues/1993#issuecomment-193921016 .

Setting the robolectric.scheduling.global system property to true will tie all loopers to the same scheduler instance, which is accessible in RuntimeEnvironment.getMasterScheduler(). The old behaviour (the default) is for each looper to have its own independent scheduler instance, which meant when you advanced them you had to advance them all individually.

Hi , it will probably end up as the default at some point, however because there is a significant body of installed code already which may depend on the old behavior we have made it optional for now, at least until we can evaluate any backward compatibility issues. It's pretty easy to make it the default for your project if that's what you want.

, I have a branch where this is almost working. See #2166 for more info. If it doesn't already work for you I'm hoping to have it ready in the next couple weeks. One tip that might help is to set your test to use a global scheduler, look in RoboSettings for more details.

However, this also means less control over what the computer is doing since you are describing ideas rather than actions.

This is exactly were we need to head towards! We just need to exchange "ideas" with "specifications" and need to trust that the machine or the used framework can find out the best way to do what we want. This should also be the default way cause "premature optimizations are the root of all evil".

I would not recommend to use the hooks api at all, if you want to be as functional as possible. You would already need to wrap your useState function in an IO monad cause this function is not pure.

already tried https://github.com/gcanti/fp-ts/issues/823#issuecomment-504086813

thanks for clarification. Seems to be the same pattern that I described only that I use a utility library for creating the new Err sum type.

Maybe I am a little bit uncool from a FP perspective :wink: but I use a wrapper union type for all possible errors I want to combine. Instead of widen you can then use a simple mapLeft.

ts
import { Union, of } from 'ts-union'

const AppError = Union({
  FsError: of<fserror>(),
  HttpError: of<httperror>()
})
type AppError = typeof AppError.T

const program: Either<apperror void> = pipe(
  get('http://purescript.org'),
  E.mapLeft(AppError.HttpError),
  E.chain(content => pipe(
    write('~/purescript.html', content),
    E.mapLeft(AppError.FsError)
  ))
)
</apperror></httperror></fserror>

What I find beneficial is that the error represents the path where the error happend because in bigger application I wrap error wrapper types in other error wrapper types. Also are these type extendable. So you can add every metadata you want.

As far as I understand, it's purpose is to avoid threading arguments through multiple functions in order only to get them where they belong (at the bottom of calls).

Thats the high level benefit. I would describe it like you can use a function without calling it or a Reader is just a monad for all functions of type (a: A) => B. The later one is important to understand then you see Readers everywhere.

Here a simple example. Please not that ReaderTaskEither<R, E, A> is just an alias for Reader<R, TaskEither<E, A>>.

ts
import * as Rr from 'fp-ts/lib/Reader'
import * as RTE from 'fp-ts/lib/ReaderTaskEither'
import * as TE from 'fp-ts/lib/TaskEither'
import * as b from 'fp-ts/lib/boolean'
import { pipe } from 'fp-ts/lib/pipeable'

interface Config {
  logLevel: number
}

const logLevel: Rr.Reader<config number> = config => config.logLevel

declare const foo: RTE.ReaderTaskEither<config error void>
const bar: RTE.ReaderTaskEither<config error void> = pipe(
  logLevel,
  Rr.map(logLevel =>
    pipe(
      logLevel > 5,
      b.fold(
        () => TE.right(undefined),
        () => TE.rightIO(() => console.log('Some side effect'))
      )
    )
  )
)

const main: RTE.ReaderTaskEither<config error void> = pipe(
  foo,
  RTE.chain(() => bar)
)

RTE.run(main, {
  logLevel: 10
})
</config></config></config></config>

+1 for the readonly modules.

If we can prevent programming errors at compile time we should go this direction or we run in the problem that the interface behavior does not match the runtime behavior.

Ok can confirm the error now. ts-node does not output any error. tsc does. Still buildable but with error output on the console. When you use fp-ts as library the signature changes as said to any type cause the type can not be found.