Skip to content

Conversation

matt-h
Copy link
Contributor

@matt-h matt-h commented Oct 2, 2020

When using a date field without time the tests need to check against a formatted date.

Otherwise when running the tests you get an error that the date string does not match a Carbon object.

I'm using this basic draft.yaml

models:
  Post:
    published_date: date

controllers:
  Post:
    resource

This update checks if the field type is a date during the test generator and includes a format. This was the simplest way I could find to check if the column was a date. If there was more than 1 exception, I could see refactoring this out to a Registry like for Faker.

I tested this against both Laravel 7 and 8

@jasonmccreary
Copy link
Collaborator

@matt-h does the generated test fail previously?

@matt-h
Copy link
Contributor Author

matt-h commented Oct 3, 2020

Yes, the test generated from this draft before this change fails.

The failures is:


1) Tests\Feature\Http\Controllers\PostControllerTest::update_redirects
Failed asserting that Illuminate\Support\Carbon Object &0000000055a7cc8e00000000776f0d2e (
    'constructedObjectId' => '0000000055a7cc8e00000000776f0d2e'
    'localMonthsOverflow' => null
    'localYearsOverflow' => null
    'localStrictModeEnabled' => null
    'localHumanDiffOptions' => null
    'localToStringFormat' => null
    'localSerializer' => null
    'localMacros' => null
    'localGenericMacros' => null
    'localFormatFunction' => null
    'localTranslator' => null
    'dumpProperties' => Array &0 (
        0 => 'date'
        1 => 'timezone_type'
        2 => 'timezone'
    )
    'dumpLocale' => null
    'date' => '1994-07-01 00:00:00.000000'
    'timezone_type' => 3
    'timezone' => 'UTC'
) matches expected '1994-07-01'.```

@jasonmccreary
Copy link
Collaborator

I'm still thinking on this. I understand it is a bug. But I'm trying to think of the convention I want to use for these cases. Said another way, what communicates the best in the test.

Simply formatting the returned date implicitly makes the comparison work for Faker. So I'm wondering if explicitly casting the faker object to Carbon and relying upon its equality check would be better.

@matt-h
Copy link
Contributor Author

matt-h commented Oct 6, 2020

I think I like setting the Faker item to Carbon here. Makes it clear that you are getting a Carbon object back from the Model and checks that the actual Carbon object matches (has no time set).

@jasonmccreary
Copy link
Collaborator

Fair. Then the generated code for the request would need to be formatted.

@matt-h
Copy link
Contributor Author

matt-h commented Oct 6, 2020

I just pushed an update to this that uses Carbon::parse in the assert statement itself. Let me know if you have a better idea how to handle this.

@jasonmccreary
Copy link
Collaborator

I do think I like this better. However, looks like we have some failing tests.

@matt-h
Copy link
Contributor Author

matt-h commented Oct 8, 2020

Looking at the test details, looks like Github Actions just failed to start for some of them.

GitHub.Services.WebApi.VssServiceResponseException: Service Unavailable
   at GitHub.Services.WebApi.VssHttpClientBase.HandleResponseAsync(HttpResponseMessage response, CancellationToken cancellationToken)
   at GitHub.Services.WebApi.VssHttpClientBase.SendAsync(HttpRequestMessage message, HttpCompletionOption completionOption, Object userState, CancellationToken cancellationToken)
   at GitHub.Services.WebApi.VssHttpClientBase.SendAsync[T](HttpRequestMessage message, Object userState, CancellationToken cancellationToken)
   at GitHub.Services.Location.Client.LocationHttpClient.GetConnectionDataAsync(ConnectOptions connectOptions, Int64 lastChangeId, CancellationToken cancellationToken, Object userState)
   at GitHub.Services.WebApi.Location.VssServerDataProvider.GetConnectionDataAsync(ConnectOptions connectOptions, Int32 lastChangeId, CancellationToken cancellationToken)
   at GitHub.Services.WebApi.Location.VssServerDataProvider.ConnectAsync(ConnectOptions connectOptions, CancellationToken cancellationToken)
   at GitHub.Runner.Common.JobServer.ConnectAsync(VssConnection jobConnection)
   at GitHub.Runner.Worker.JobRunner.RunAsync(AgentJobRequestMessage message, CancellationToken jobRequestCancellationToken)
   at GitHub.Runner.Worker.Worker.RunAsync(String pipeIn, String pipeOut)
   at GitHub.Runner.Worker.Program.MainAsync(IHostContext context, String[] args)

And

GitHub Actions has encountered an internal error when running your job.

Are you able to just have it retry the tests?

@matt-h
Copy link
Contributor Author

matt-h commented Oct 8, 2020

Looks like the re-run fixed it.

@jasonmccreary jasonmccreary merged commit bcdd8ff into laravel-shift:master Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants