Skip to content

Commit f848420

Browse files
authored
Merge pull request #1011 from andrewrabert/fix_base_url
Fix base URL determination
2 parents d5a43b9 + 5f015b9 commit f848420

9 files changed

Lines changed: 568 additions & 100 deletions

File tree

.github/workflows/test.yml

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
name: test
2+
3+
on:
4+
pull_request:
5+
push:
6+
branches:
7+
- master
8+
9+
jobs:
10+
test:
11+
runs-on: ubuntu-latest
12+
steps:
13+
- uses: actions/checkout@v4
14+
15+
- name: Install build dependencies from debian/control
16+
run: |
17+
sudo apt-get update
18+
sudo apt-get install --yes devscripts equivs
19+
sudo mk-build-deps -i -r -t "apt-get --yes" debian/control
20+
21+
- name: Configure
22+
run: cmake -B build -DCMAKE_BUILD_TYPE=Debug
23+
24+
- name: Build
25+
run: cmake --build build
26+
27+
- name: Run tests
28+
run: |
29+
cd build
30+
ctest --output-on-failure

CMakeLists.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,9 @@ endif(Qt5_POSITION_INDEPENDENT_CODE)
7777
add_subdirectory(external)
7878
add_subdirectory(src)
7979

80+
enable_testing()
81+
add_subdirectory(tests)
82+
8083
include(CPackConfiguration)
8184

8285
add_custom_target(install_app_bundle COMMAND ${CMAKE_COMMAND} -P cmake_install.cmake DEPENDS JellyfinMediaPlayer)

CONTRIBUTING.md

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
# Contributing to Jellyfin Media Player
2+
3+
## Running Tests
4+
5+
Jellyfin Media Player uses Qt Test for unit testing.
6+
7+
### Building Tests
8+
9+
Tests are built automatically when you build the project:
10+
11+
```sh
12+
cmake -B build
13+
cmake --build build
14+
```
15+
16+
### Running Tests
17+
18+
Run all tests using CTest:
19+
20+
```sh
21+
cd build
22+
ctest
23+
```
24+
25+
Or run individual test executables directly:
26+
27+
```sh
28+
cd build
29+
./tests/test_systemcomponent
30+
```
31+
32+
### Writing Tests
33+
34+
Tests are located in the `tests/` directory. To add a new test:
35+
36+
1. Create a test file in `tests/` (e.g., `test_mycomponent.cpp`)
37+
2. Use `QTEST_APPLESS_MAIN` for headless unit tests
38+
3. Add the test to `tests/CMakeLists.txt`
39+
40+
Example test structure:
41+
42+
```cpp
43+
#include <QtTest/QtTest>
44+
#include "../src/mycomponent/MyComponent.h"
45+
46+
class TestMyComponent : public QObject
47+
{
48+
Q_OBJECT
49+
50+
private slots:
51+
void testMyFunction_data();
52+
void testMyFunction();
53+
};
54+
55+
void TestMyComponent::testMyFunction_data()
56+
{
57+
QTest::addColumn<QString>("input");
58+
QTest::addColumn<QString>("expected");
59+
60+
QTest::newRow("test case 1") << "input1" << "output1";
61+
QTest::newRow("test case 2") << "input2" << "output2";
62+
}
63+
64+
void TestMyComponent::testMyFunction()
65+
{
66+
QFETCH(QString, input);
67+
QFETCH(QString, expected);
68+
69+
QString result = MyComponent::myFunction(input);
70+
QCOMPARE(result, expected);
71+
}
72+
73+
QTEST_APPLESS_MAIN(TestMyComponent)
74+
#include "test_mycomponent.moc"
75+
```
76+
77+
For more information on Qt Test, see the [Qt Test documentation](https://doc.qt.io/qt-6/qtest-overview.html).

native/connectivityHelper.js

Lines changed: 34 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
window.jmpCheckServerConnectivity = (() => {
2-
let checkInProgress = false;
2+
let activeController = null;
33

4-
return async function(url) {
5-
if (checkInProgress) {
6-
throw new Error('Connectivity check already in progress');
4+
const checkFunc = async function(url) {
5+
// Abort any in-progress check
6+
if (activeController) {
7+
activeController.abort();
78
}
89

910
// Wait for API
@@ -16,24 +17,48 @@ window.jmpCheckServerConnectivity = (() => {
1617
throw new Error('WebChannel not available');
1718
}
1819

19-
checkInProgress = true;
20+
// Create abort controller for this check
21+
const controller = new AbortController();
22+
activeController = controller;
2023

2124
return new Promise((resolve, reject) => {
22-
const handler = (resultUrl, success) => {
23-
if (resultUrl === url) {
25+
// Handle abort
26+
controller.signal.addEventListener('abort', () => {
27+
if (handler) {
2428
window.api.system.serverConnectivityResult.disconnect(handler);
25-
checkInProgress = false;
29+
}
30+
reject(new Error('Connection cancelled'));
31+
});
32+
33+
let handler = (resultUrl, success, resolvedUrl) => {
34+
if (resultUrl === url && !controller.signal.aborted) {
35+
window.api.system.serverConnectivityResult.disconnect(handler);
36+
handler = null;
37+
if (activeController === controller) {
38+
activeController = null;
39+
}
2640
if (success) {
27-
resolve();
41+
resolve(resolvedUrl);
2842
} else {
2943
reject(new Error('Connection failed'));
3044
}
3145
}
3246
};
47+
3348
window.api.system.serverConnectivityResult.connect(handler);
3449
window.api.system.checkServerConnectivity(url);
3550
});
3651
};
52+
53+
// Expose abort function for cancellation
54+
checkFunc.abort = () => {
55+
if (activeController) {
56+
activeController.abort();
57+
activeController = null;
58+
}
59+
};
60+
61+
return checkFunc;
3762
})();
3863

3964
window.jmpFetchPage = (() => {

native/find-webclient.js

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,18 @@ async function tryConnect(server) {
33
if (!server.startsWith("http")) {
44
server = "http://" + server;
55
}
6-
serverBaseURL = server.replace(/\/+$/, "");
76

87
console.log("Checking connectivity to:", server);
98

10-
await window.jmpCheckServerConnectivity(server);
9+
const resolvedUrl = await window.jmpCheckServerConnectivity(server);
1110
console.log("Server connectivity check passed");
11+
console.log("Resolved URL:", resolvedUrl);
12+
13+
// Save original URL but navigate to fully-resolved redirect
1214
window.jmpInfo.settings.main.userWebClient = server;
13-
window.location = server;
15+
16+
// Navigation will clean up handlers, but do it explicitly
17+
window.location = resolvedUrl;
1418

1519
return true;
1620
} catch (e) {
@@ -54,16 +58,8 @@ const startConnecting = async () => {
5458
button.style.visibility = 'hidden';
5559
document.addEventListener('keydown', cancelOnEscape);
5660

57-
let connected = false;
58-
59-
while (!connected && isConnecting) {
60-
connected = await tryConnect(server);
61-
62-
if (!connected && isConnecting) {
63-
// Wait 5 seconds before retrying
64-
await new Promise(resolve => setTimeout(resolve, 5000));
65-
}
66-
}
61+
// C++ handles retries, just wait for result
62+
const connected = await tryConnect(server);
6763

6864
if (!connected) {
6965
isConnecting = false;
@@ -82,8 +78,17 @@ const startConnecting = async () => {
8278
const cancelConnection = () => {
8379
if (!isConnecting) return;
8480

81+
console.log("Cancelling connection");
8582
isConnecting = false;
8683

84+
// Cancel C++ connectivity check and abort JS promise
85+
if (window.api && window.api.system) {
86+
window.api.system.cancelServerConnectivity();
87+
}
88+
if (window.jmpCheckServerConnectivity.abort) {
89+
window.jmpCheckServerConnectivity.abort();
90+
}
91+
8792
const address = document.getElementById('address');
8893
const title = document.getElementById('title');
8994
const spinner = document.getElementById('spinner');
@@ -160,16 +165,8 @@ document.addEventListener('keydown', (e) => {
160165
button.style.visibility = 'hidden';
161166
document.addEventListener('keydown', cancelOnEscape);
162167

163-
let connected = false;
164-
165-
while (!connected && isConnecting) {
166-
connected = await tryConnect(savedServer);
167-
168-
if (!connected && isConnecting) {
169-
// Wait 5 seconds before retrying
170-
await new Promise(resolve => setTimeout(resolve, 5000));
171-
}
172-
}
168+
// C++ handles retries, just wait for result
169+
const connected = await tryConnect(savedServer);
173170

174171
if (!connected) {
175172
// User cancelled or error - show UI

0 commit comments

Comments
 (0)