-
Notifications
You must be signed in to change notification settings - Fork 43
test: fix relative paths in test cases #325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,9 +16,11 @@ class TestDesktopEntry : public testing::Test | |
| static void SetUpTestCase() | ||
| { | ||
| env = qgetenv("XDG_DATA_DIRS"); | ||
| auto curDir = QDir::current(); | ||
| QByteArray fakeXDG = (curDir.absolutePath() + QDir::separator() + "data").toLocal8Bit(); | ||
| ASSERT_TRUE(qputenv("XDG_DATA_DIRS", fakeXDG)); | ||
| auto fakeXDG = QDir::current(); | ||
| ASSERT_TRUE(fakeXDG.cdUp()); | ||
| ASSERT_TRUE(fakeXDG.cdUp()); | ||
| ASSERT_TRUE(fakeXDG.cd("tests/data")); | ||
| ASSERT_TRUE(qputenv("XDG_DATA_DIRS", fakeXDG.absolutePath().toLocal8Bit())); | ||
| ParserError err; | ||
| auto file = DesktopFile::searchDesktopFileById("deepin-editor", err); | ||
| if (!file.has_value()) { | ||
|
|
@@ -42,9 +44,11 @@ TEST_F(TestDesktopEntry, desktopFile) | |
| const auto &fileptr = file(); | ||
| ASSERT_FALSE(fileptr.isNull()); | ||
| const auto &exampleFile = file(); | ||
| auto curDir = QDir::current(); | ||
| QString path{curDir.absolutePath() + QDir::separator() + "data" + QDir::separator() + "applications" + QDir::separator() + | ||
| "deepin-editor.desktop"}; | ||
| auto desktopFileDir = QDir::current(); | ||
| ASSERT_TRUE(desktopFileDir.cdUp()); | ||
| ASSERT_TRUE(desktopFileDir.cdUp()); | ||
| ASSERT_TRUE(desktopFileDir.cd("tests/data/applications")); | ||
| const auto& path = desktopFileDir.absoluteFilePath("deepin-editor.desktop"); | ||
|
Comment on lines
+47
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Reuse the same data-root resolution as in This test now resolves the path differently from Suggested implementation: const auto &fileptr = file();
ASSERT_FALSE(fileptr.isNull());
const auto &exampleFile = file();
// Resolve the desktop file path based on the same data-root used to set XDG_DATA_DIRS.
QDir applicationsDir{fakeXDG};
ASSERT_TRUE(applicationsDir.cd("applications"));
const auto &path = applicationsDir.absoluteFilePath("deepin-editor.desktop");
EXPECT_EQ(exampleFile->sourcePath(), path);
EXPECT_EQ(exampleFile->desktopId().toStdString(), "deepin-editor");
}
If |
||
| EXPECT_EQ(exampleFile->sourcePath(), path); | ||
| EXPECT_EQ(exampleFile->desktopId().toStdString(), "deepin-editor"); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,14 +3,18 @@ | |
| // SPDX-License-Identifier: LGPL-3.0-or-later | ||
|
|
||
| #include "applicationHooks.h" | ||
| #include <gtest/gtest.h> | ||
| #include <QDir> | ||
| #include <QStringList> | ||
| #include <gtest/gtest.h> | ||
|
|
||
| TEST(ApplicationHookTest, load) | ||
| { | ||
| auto file = | ||
| QDir::currentPath() + QDir::separator() + "data" + QDir::separator() + "hooks.d" + QDir::separator() + "1-test.json"; | ||
| auto hooksDir = QDir::current(); | ||
| ASSERT_TRUE(hooksDir.cdUp()); | ||
| ASSERT_TRUE(hooksDir.cdUp()); | ||
| ASSERT_TRUE(hooksDir.cd("tests/data/hooks.d")); | ||
|
|
||
| auto file = hooksDir.absoluteFilePath("1-test.json"); | ||
|
Comment on lines
+12
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Make hook test data lookup resilient to different working directories (e.g. via a shared helper or QFINDTESTDATA). This test currently assumes the process starts two levels below the project root and that Suggested implementation: #include "applicationHooks.h"
#include <gtest/gtest.h>
#include <QDir>
#include <QStringList>
#include <QtTest/QTest>
TEST(ApplicationHookTest, load)
{
const auto file = QFINDTESTDATA("data/hooks.d/1-test.json");
ASSERT_FALSE(file.isEmpty());
auto hook = ApplicationHook::loadFromFile(QString::fromUtf8(file));
To fully support this change, ensure that |
||
| auto hook = ApplicationHook::loadFromFile(file); | ||
| EXPECT_TRUE(hook); | ||
| EXPECT_EQ(hook->hookName(), QString{"1-test.json"}); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (testing): Consider using a shared helper or QFINDTESTDATA-style lookup for the test data root instead of hardcoding
cdUp()hops.These
cdUp()calls still rely on a fixed directory layout (two levels up, thentests/data), which can break under different IDE/CTest/build setups. Centralizing the lookup in a helper (e.g.QDir testDataRoot()) or usingQFINDTESTDATAwould make the tests more portable and avoid duplicating this logic later in the file.Suggested implementation:
auto desktopFileDir = testDataRoot();<QDir><QString><QTest>(or a header that provides theQFINDTESTDATAmacro).QFINDTESTDATAin this compilation unit, either:#ifdef QFINDTESTDATAblock and rely solely on the fallback directory walk as appropriate for your environment.tests/dataor uses a different relative structure, update themarkerconstant intestDataRoot()accordingly so it resolves the correct directory.