Skip to content
This repository has been archived by the owner on Mar 11, 2021. It is now read-only.

WIP: mechanics to strip down golden files #2218

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

kwk
Copy link
Collaborator

@kwk kwk commented Aug 6, 2018

Purpose

This adds the mechanics to create golden files with only a portion of the original golden file in it.

Background

Sometimes you might be only interested in only a special object within a golden file and that is when you can give an optional JSON path expression to controller.compareWithGolden[Agnostic|AgnosticUUID|AgnosticTime]().

Example

For example, if your golden file looks like this:

{
  "data": {
    "attributes": {
      "system.title": "foobar"
    }
  }
}

but you're only interested in the attributes system.title you can give this expression

$.data.attributes["system.title"]

which results in a golden file with just "foobar" inside.

Please note that you have to use the square brackets notation if the key you're trying to access has a dot inside its name. The syntax here slightly deviates from the original JSON Path syntax in that you have to use double instead of single quotes because of an implementation detail of the library that we use.

Pros and Cons

We have golden files to document side-effects of changes in our code and to have an example response for all the endpoints that we support (we don't have it for all at the moment).

We should keep this and not strip down all the golden files. But let's say you have the output when a work item is being updated in a golden file once. You don't need the whole response for tests that check if you can update a custom field in that work item.

Additional changes

Apart from the above this change

  • "hides" some of the internal functions for golden files like replaceUUIDs or replaceTimes by prefixing them with a and underscore.

  • strips down the test output for controller/TestSuiteWorkItem2/TestWI2UpdateFieldOfDifferentSimpleTypes to only contain the field value from the the JSON output.

  • removes the need to replace UUIDs, replace times from the expected string and have strip down to some pattern. That is being done when the file is written when the -update flag is given already. Therefore there's no need to do it twice.

@kwk kwk self-assigned this Aug 6, 2018
@kwk kwk changed the title WIP: slim golden files WIP: mechanics to have slim golden files Aug 10, 2018
@codecov-io
Copy link

codecov-io commented Aug 10, 2018

Codecov Report

Merging #2218 into master will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2218      +/-   ##
==========================================
+ Coverage   69.85%   69.87%   +0.01%     
==========================================
  Files         170      170              
  Lines       15872    15872              
==========================================
+ Hits        11088    11090       +2     
+ Misses       3770     3769       -1     
+ Partials     1014     1013       -1
Impacted Files Coverage Δ
controller/workitem.go 80.03% <0%> (+0.39%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e258355...c1cf26e. Read the comment docs.

@kwk kwk changed the title WIP: mechanics to have slim golden files WIP: mechanics to slim golden files Aug 10, 2018
@kwk
Copy link
Collaborator Author

kwk commented Aug 10, 2018

[test]

@kwk kwk requested a review from jarifibrahim August 10, 2018 07:27
@kwk kwk changed the title WIP: mechanics to slim golden files mechanics to slim golden files Aug 10, 2018
@kwk kwk changed the title mechanics to slim golden files mechanics to strip down golden files Aug 10, 2018
@kwk kwk requested a review from aslakknutsen August 10, 2018 07:48
@kwk kwk changed the title mechanics to strip down golden files WIP: mechanics to strip down golden files Aug 10, 2018
@@ -81,6 +87,9 @@ func testableCompareWithGolden(update bool, goldenFile string, actualObj interfa
if err != nil {
return errs.WithStack(err)
}
if len(stripDownToPattern) > 1 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kwk Why are accepting []strings (stripDownToPattern ...string) in function and then checking it's length to be 1? If this is the case, shouldn't the stripDownToPattern be a string and not ...string?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is an optional parameter for now because your don't always want to strip down your golden file. And since we cannot handle more than one pattern now, I check for the length to be 1.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So why not change ...string to just string ? We can later update this to support multiple patterns

Copy link
Member

@jarifibrahim jarifibrahim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if we want this change. If the idea is to compare only one value, why should we even use a golden file. We could do this with a simple assert.Equal

@kwk
Copy link
Collaborator Author

kwk commented Aug 13, 2018

If the idea is to compare only one value, why should we even use a golden file

The idea is NOT to compare just one value but a subtree.

@kwk kwk added the idea An idea is a PR that shall not be deleted and captures a brain dump of some sort. label Aug 23, 2018
@stooke stooke mentioned this pull request Sep 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🛀 cleanup 💣 test idea An idea is a PR that shall not be deleted and captures a brain dump of some sort. jsonapi ♻️ refactoring work-in-progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants