tests: update/add tests for inspire-schemas v4.0 #1903

spirosdelviniotis · 2017-02-01T16:44:00Z

Signed-off-by: Spiros Delviniotis spyridon.delviniotis@cern.ch

jacquerie · 2017-02-02T08:11:33Z

tests/unit/dojson/test_dojson_hep.py

+    result = hep.do(create_record(snippet))
+
+    assert validate(result['isbns'], subschema) is None
+    assert expected == result['isbns'][0]


assert expected == result['isbns']

and above

expected = [ { ... }, ]

jacquerie · 2017-02-02T08:12:25Z

tests/unit/dojson/test_dojson_hep.py

@@ -44,9 +49,20 @@ def test_doi_but_should_be_hdl_from_0247_a():
    }]
    result = hep.do(create_record(snippet))

+    assert validate(result['persistent_identifiers'], subschema) is None
    assert result.get('persistent_identifiers', []) == expected
    assert result.get('dois', []) == []


assert expected == result['persistent_identifiers']

jacquerie · 2017-02-02T08:26:57Z

tests/unit/dojson/test_dojson_hep.py

+    assert expected == result['020']
+
+
+def test_isbns_from_020():


test_isbns_from_020__a_b_normalizes_print

jacquerie · 2017-02-02T08:28:47Z

tests/unit/dojson/test_dojson_hep.py

+        '<datafield tag="020" ind1=" " ind2=" ">'
+        '  <subfield code="a">9780198759713</subfield>'
+        '</datafield>'
+    )  # record/1510325/export/xm


/export/xm is implied by the fact that this is MARCXML, so it can be dropped here.

jacquerie · 2017-02-02T08:29:03Z

tests/unit/dojson/test_dojson_hep.py

+    assert expected == result['773']
+
+
+def test_isbns_from_020_a_only():


test_isbns_from_020__a

jacquerie · 2017-02-02T08:36:02Z

Will be merged in #1899 when the comments are addressed.

spirosdelviniotis · 2017-02-02T15:21:37Z

tests/unit/dojson/test_dojson_hep.py

+    ]
+    result = hep2marc.do(result)
+
+    assert expected == result['245']


@kaplun @jacquerie Do we need this case?
As I realized, we don't have dojson rule from json to MARCXML for 247 field.

On production we only have 6(!) records with 247. @michamos I think this are actually outliers and they should have used 246, right? (In the TWiki it says deprecated but to me looks like the other way round).

jacquerie · 2017-02-04T07:53:58Z

tests/unit/dojson/test_dojson_hep.py

+        {
+            'c': ['45'],
+            'p': 'IAU Symp.',
+            '0': 1408366,


I don't think we should be producing guys like these, because they come from the xme format, while here we are producing the xm format. This is a common problem in the MARCXML -> JSON rules as they are currently written.

CC: @kaplun

@jacquerie : you are right. In general we should not export back these records IDs.

Result of the discussion with @kaplun: we should write in MARCXML all IDs that exist in the XM format (those not marked XME only in https://twiki.cern.ch/twiki/bin/view/Inspire/DevelopmentRecordMarkup )

jacquerie · 2017-02-05T00:12:43Z

tests/unit/dojson/test_dojson_hep.py

+            'keyword': 'programming: Monte Carlo',
+            'classification_scheme': 'INSPIRE',
+        },
+    ]


Watch out! Because you are not wrapping the two <datafield> tags with a <record> tag create_record only outputs the first.

jacquerie · 2017-02-06T09:55:12Z

These tests need to be split following what was done in #1899. @michamos, can you check them?

spirosdelviniotis · 2017-02-06T11:12:31Z

Just re based !

michamos · 2017-02-06T12:01:50Z

tests/unit/dojson/test_dojson_hep_bd2xx.py

@@ -136,3 +136,37 @@ def test_titles_from_245__a_b():
    result = hep2marc.do(result)

    assert expected == result['245']
+
+
+def test_titles_from_245_and_247__a_9():


we only had 6 records in HEP with 247 content, which I all changed to 247, so this test can be removed.

michamos · 2017-02-06T12:04:21Z

tests/unit/dojson/test_dojson_hep_bd2xx.py

+    ]
+    result = hep2marc.do(result)
+
+    assert expected == result['245']


You should add a test for the case where the two titles in 245 and 246 are different, e.g. http://inspirehep.net/record/36035. In that case, the 245 goes to the first element of titles, the 246s go next.

actually, forget about this, just wrap everything with more than one <datafield> into a <record> and all titles should be preserved.

michamos · 2017-02-06T12:08:07Z

tests/unit/dojson/test_dojson_hep_bd2xx.py

@@ -170,3 +170,68 @@ def test_titles_from_245_and_247__a_9():
    result = hep2marc.do(result)

    assert expected == result['245']
+
+
+def test_title_translations_from_242__a():


This test is wrong: if there is a 242, 245 goes to translated_titles with the right language, whereas 242 goes to titles.

michamos · 2017-02-06T12:08:27Z

tests/unit/dojson/test_dojson_hep_bd2xx.py

+    assert expected == result['242']
+
+
+def test_title_translations_from_242__a_b():


Same comment.

michamos · 2017-02-06T12:11:44Z

tests/unit/dojson/test_dojson_hep_bd01x09x.py

+        '</datafield>'
+    )  # record/26564
+
+    expected = [


You shouldn't discard report numbers! report_numbers is a list for a good reason: so that we can place several values inside 😉
You should put both in expected and make sure they round-trip.

michamos · 2017-02-06T12:13:47Z

tests/unit/dojson/test_dojson_hep_bd6xx.py

+        '</datafield>'
+    )  # record/363605
+
+    expected = [


again, you are discarding half of the keywords.

Signed-off-by: Spiros Delviniotis <spyridon.delviniotis@cern.ch>

michamos · 2017-02-06T16:54:41Z

@jacquerie @kaplun LGTM

spirosdelviniotis added the WIP label Feb 1, 2017

jacquerie reviewed Feb 2, 2017

View reviewed changes

spirosdelviniotis force-pushed the inspire_next_move_to_schemas_v4 branch 4 times, most recently from b3deef2 to ea91f8e Compare February 2, 2017 15:17

spirosdelviniotis commented Feb 2, 2017

View reviewed changes

spirosdelviniotis force-pushed the inspire_next_move_to_schemas_v4 branch from ea91f8e to 8d3fb5c Compare February 2, 2017 16:11

jacquerie reviewed Feb 4, 2017

View reviewed changes

jacquerie reviewed Feb 5, 2017

View reviewed changes

spirosdelviniotis force-pushed the inspire_next_move_to_schemas_v4 branch from 0eaf664 to 2e614a6 Compare February 6, 2017 11:11

michamos requested changes Feb 6, 2017

View reviewed changes

spirosdelviniotis force-pushed the inspire_next_move_to_schemas_v4 branch 3 times, most recently from 9bc6fc2 to 18feab5 Compare February 6, 2017 13:48

spirosdelviniotis added Need: Review and removed WIP labels Feb 6, 2017

spirosdelviniotis force-pushed the inspire_next_move_to_schemas_v4 branch 2 times, most recently from 0111ac7 to b2c0a91 Compare February 6, 2017 15:39

spirosdelviniotis added 4 commits February 6, 2017 17:44

tests: add tests for 245 and 246 fields

32d92e9

Signed-off-by: Spiros Delviniotis <spyridon.delviniotis@cern.ch>

tests: add tests for 242 field

bc74b99

Signed-off-by: Spiros Delviniotis <spyridon.delviniotis@cern.ch>

tests: add tests for 037 field

f91b7e3

Signed-off-by: Spiros Delviniotis <spyridon.delviniotis@cern.ch>

tests: add tests for 695 and 653 fields

c5cdac6

Signed-off-by: Spiros Delviniotis <spyridon.delviniotis@cern.ch>

spirosdelviniotis force-pushed the inspire_next_move_to_schemas_v4 branch from b2c0a91 to c5cdac6 Compare February 6, 2017 16:46

michamos approved these changes Feb 6, 2017

View reviewed changes

kaplun merged commit ca54db1 into inspirehep:master Feb 7, 2017

kaplun removed the Need: Review label Feb 7, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: update/add tests for inspire-schemas v4.0 #1903

tests: update/add tests for inspire-schemas v4.0 #1903

spirosdelviniotis commented Feb 1, 2017

jacquerie Feb 2, 2017

jacquerie Feb 2, 2017

jacquerie Feb 2, 2017

jacquerie Feb 2, 2017

jacquerie Feb 2, 2017

jacquerie commented Feb 2, 2017

spirosdelviniotis Feb 2, 2017 •

edited

kaplun Feb 3, 2017

jacquerie Feb 4, 2017

kaplun Feb 5, 2017

michamos Feb 6, 2017 •

edited

jacquerie Feb 5, 2017

jacquerie commented Feb 6, 2017

spirosdelviniotis commented Feb 6, 2017

michamos Feb 6, 2017

michamos Feb 6, 2017

michamos Feb 6, 2017

michamos Feb 6, 2017

michamos Feb 6, 2017

michamos Feb 6, 2017

michamos Feb 6, 2017

michamos commented Feb 6, 2017

		assert expected == result['773']


		def test_isbns_from_020_a_only():

		assert expected == result['242']


		def test_title_translations_from_242__a_b():

tests: update/add tests for inspire-schemas v4.0 #1903

tests: update/add tests for inspire-schemas v4.0 #1903

Conversation

spirosdelviniotis commented Feb 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacquerie commented Feb 2, 2017

spirosdelviniotis Feb 2, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michamos Feb 6, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacquerie commented Feb 6, 2017

spirosdelviniotis commented Feb 6, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michamos commented Feb 6, 2017

spirosdelviniotis Feb 2, 2017 •

edited

michamos Feb 6, 2017 •

edited