Skip to content

Commit

Permalink
Added test for isNull/isValid returns
Browse files Browse the repository at this point in the history
  • Loading branch information
@elpaso
elpaso committed May 27, 2019
1 parent d7019ce commit 403bacf
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 5 deletions.
2 changes: 1 addition & 1 deletion src/core/qgsjsonutils.cpp
View file
Expand Up @@ -347,7 +347,7 @@ QVariantList QgsJsonUtils::parseArray( const QString &json, QVariant::Type type
}
else if ( item.is_null() )
{
// do nothing: the default is null
v = QVariant( type );
}
else
{
Expand Down
13 changes: 13 additions & 0 deletions tests/src/core/testqgsjsonutils.cpp
View file
Expand Up @@ -92,6 +92,19 @@ void TestQgsJsonUtils::testJsonArray()
QCOMPARE( QgsJsonUtils::parseArray( "", QVariant::Int ), QVariantList() );
// Nulls
QCOMPARE( QgsJsonUtils::parseArray( R"([null, null])" ), QVariantList() << QVariant() << QVariant() );
QVERIFY( QVariant().isValid() );
for ( const QVariant &value : QgsJsonUtils::parseArray( R"([null, null])" ) )
{
QVERIFY( value.isNull() );
// This would fail because the expected type is not set and an invalid QVariant is created:
// QVERIFY( value.isValid() );

This comment has been minimized.

Sign in to view

Copy link
@m-kuhn

m-kuhn May 27, 2019

Member

I'd prefer to be consistent.
Either always return NULL or always return Invalid.
IMHO the NULL vs. Invalid information is more important than the type information for NULL values.

Short reasoning: Database systems also don't care about types of NULL (i.e. one always writes INSERT INTO ... VALUES ( NULL ) without specifying a type. But there's a difference between NULL and no value (e.g. when doing an INSERT, NULL will insert NULL whereas omitting a value will trigger the default value). The example is not very applicable here, but I think it makes sense to have this behavior as what we intend consistently.
}
QCOMPARE( QgsJsonUtils::parseArray( R"([null, null])", QVariant::Type::Int ), QVariantList() << QVariant( QVariant::Type::Int ) << QVariant( QVariant::Type::Int ) );
for ( const QVariant &value : QgsJsonUtils::parseArray( R"([null, null])", QVariant::Type::Int ) )
{
QVERIFY( value.isNull() );
QVERIFY( value.isValid() );
}
}

void TestQgsJsonUtils::testIntList()
Expand Down
6 changes: 2 additions & 4 deletions tests/src/gui/testqgsvaluerelationwidgetwrapper.cpp
View file
Expand Up @@ -878,12 +878,10 @@ void TestQgsValueRelationWidgetWrapper::testWithJsonInSpatialiteTextFk()
vl_json->changeAttributeValue( 1, fk_field_idx, w_favoriteauthors.value() );
// check if stored correctly
vl_json->commitChanges();
QVariantList expected_vl;
expected_vl << "1gamma" << "2helm,comma" << "3johnson\"quote" << "5adams'singlequote";
QgsFeature f = vl_json->getFeature( 1 );
QVariant attribute = f.attribute( fk_field );
QList<QVariant> value = attribute.toList();
QCOMPARE( value, expected_vl );
QVariantList value = attribute.toList();
QCOMPARE( value, QVariantList( { "1gamma", "2helm,comma", "3johnson\"quote", "5adams'singlequote" } ) );

// FEATURE 2
w_favoriteauthors.setFeature( vl_json->getFeature( 2 ) );
Expand Down

0 comments on commit 403bacf

Please sign in to comment.