Fix history state index handling

The history state is not present in the generated statemachines,
and the states following a history state have a state index that is
lower than what the actual statemachine considers. This patch adjusts
the index calculation on the statemachine side.

The scion test suite history tests passed because it used the
activeStateNames() method for verifying which states are active.
That method worked. This commit also now amends the scion
verification to cover the metaobject access and (de-)activation
signal emissions; the scion history tests would now fail without
the index handling adaptation of this patch.

Fixes: QTBUG-80262
Change-Id: Ifc509794bf3dca6c81e969e8a4e2bbaf9618d275
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
(cherry picked from commit 50815b095f1ccea936c89964e390e7e774efbf4b)

2 files changed, 56 insertions, 7 deletions

diff --git a/src/scxml/qscxmlstatemachine.cpp b/src/scxml/qscxmlstatemachine.cpp
index 626cdca..4adbe4b 100644
--- a/src/scxml/qscxmlstatemachine.cpp
+++ b/src/scxml/qscxmlstatemachine.cpp
@@ -767,6 +767,9 @@ void QScxmlStateMachinePrivate::attach(QScxmlStateMachineInfo *info)
 
 void QScxmlStateMachinePrivate::updateMetaCache()
 {
+    // This function creates a mapping from state index/name to their signal indexes.
+    // The state index may differ from its signal index as we don't generate history
+    // and invalid states, effectively skipping them
     m_stateIndexToSignalIndex.clear();
     m_stateNameToSignalIndex.clear();
 
@@ -2366,7 +2369,11 @@ void QScxmlStateMachine::stop()
 bool QScxmlStateMachine::isActive(int stateIndex) const
 {
     Q_D(const QScxmlStateMachine);
-    return d->m_configuration.contains(stateIndex);
+    // Here we need to find the actual internal state index that corresponds with the
+    // index of the compiled metaobject (which is same as its mapped signal index).
+    // See updateMetaCache()
+    const int mappedStateIndex = d->m_stateIndexToSignalIndex.key(stateIndex, -1);
+    return d->m_configuration.contains(mappedStateIndex);
 }
 
 QT_END_NAMESPACE
diff --git a/tests/auto/scion/tst_scion.cpp b/tests/auto/scion/tst_scion.cpp
index d1e4873..eaa8e4d 100644
--- a/tests/auto/scion/tst_scion.cpp
+++ b/tests/auto/scion/tst_scion.cpp
@@ -269,16 +269,37 @@ void TestScion::compiled()
 
 static bool verifyStates(QScxmlStateMachine *stateMachine, const QJsonObject &stateDescription, const QString &key, int counter)
 {
+    const auto errorMessage = [&key, &counter](const QStringList& current,
+                                     const QStringList& expected, const QLatin1String& details) {
+        qWarning("Incorrect %s (%d)!", qPrintable(key), counter);
+        qWarning() << "Current configuration:" << current;
+        qWarning() << "Expected configuration:" << expected;
+        qWarning() << "Failed state read was done with:" << details;
+    };
+
+    // Verify that activeStateNames() matches the expectation
     auto current = stateMachine->activeStateNames();
     std::sort(current.begin(), current.end());
     auto expected = getStates(stateDescription, key);
-    if (current == expected)
-        return true;
+    if (current != expected) {
+        errorMessage(current, expected, QLatin1String("activeStateNames()"));
+        return false;
+    }
 
-    qWarning("Incorrect %s (%d)!", qPrintable(key), counter);
-    qWarning() << "Current configuration:" << current;
-    qWarning() << "Expected configuration:" << expected;
-    return false;
+    for (const auto& s : expected) {
+        // Verify that isActive(stateName) matches the expectation
+        if (!stateMachine->isActive(s)) {
+            errorMessage(current, expected, QLatin1String("isActive()"));
+            return false;
+        }
+        // Verify that the metaobject matches the expectation
+        const auto mo = stateMachine->metaObject();
+        if (!mo->property(mo->indexOfProperty(s.toLocal8Bit())).read(stateMachine).toBool()) {
+            errorMessage(current, expected, QLatin1String("metaobject read"));
+            return false;
+        }
+    }
+    return true;
 }
 
 static bool playEvent(QScxmlStateMachine *stateMachine, const QJsonObject &eventDescription, int counter)
@@ -344,11 +365,32 @@ static bool playEvent(QScxmlStateMachine *stateMachine, const QJsonObject &event
     });
     MySignalSpy triggerSpy(&trigger, SIGNAL(timeout()));
 
+    // Create a signal spy for each state we expect to be (de)activated in next transition.
+    const auto expectedActive = getStates(eventDescription, QLatin1String("nextConfiguration"));
+    std::list<std::unique_ptr<QSignalSpy>> stateSpies;
+    const auto mo = stateMachine->metaObject();
+
+    for (const auto& s : stateMachine->stateNames()) {
+        if (expectedActive.contains(s) == stateMachine->isActive(s))
+            continue;
+        // The state is expected to undergo an activation change, create a signal spy
+        const auto changedSignal = s + QLatin1String("Changed(bool)");
+        auto signalIndex = mo->indexOfSignal(changedSignal.toUtf8().constData());
+        stateSpies.push_back(std::unique_ptr<QSignalSpy>(
+                                 new QSignalSpy(stateMachine, mo->method(signalIndex))));
+    }
+
     stateMachine->submitEvent(e);
 
     if (!triggerSpy.fastWait()) {
         qWarning() << "State machine did not reach a stable state.";
     } else if (verifyStates(stateMachine, eventDescription, QLatin1String("nextConfiguration"), counter)) {
+        for (const auto& spy : stateSpies) {
+            if (spy->size() != 1) {
+                qWarning() << "Signal error for:" << spy->signal() << ", size:" << spy->size();
+                return false;
+            }
+        }
         return true;
     }