Skip to content

Commit da0146c

Browse files
esafakchaokunyang
andcommitted
fix(ci): Exit with subprocess return code in run_ci.py (#2560)
CI was not reflecting failures in shell script fallbacks (we are migrating to python) * Use sys.exit() to propagate the return code from subprocess.call(). * This ensures that the CI pipeline correctly reflects the success or failure of the executed scripts. You might want to merge #2561 first, to reduce the number of errors. --------- Co-authored-by: chaokunyang <shawn.ck.yang@gmail.com>
1 parent 698f30c commit da0146c

31 files changed

Lines changed: 342 additions & 148 deletions

File tree

.github/pull_request_template.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ Contribution Checklist
1010
- Fory has a strong focus on performance. If the PR you submit will have an impact on performance, please benchmark it first and provide the benchmark result here.
1111
-->
1212

13+
## Why?
14+
15+
## What does this PR do?
16+
17+
<!-- Describe the purpose of this PR. -->
18+
1319
## What does this PR do?
1420

1521
<!-- Describe the purpose of this PR. -->

.github/workflows/ci.yml

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ jobs:
6262
run: python ./ci/run_ci.py cpp --install-deps-only
6363
- name: Install python dependencies
6464
run: pip install pyarrow==15.0.0 Cython wheel pytest setuptools -U
65+
- name: Install pyfory for xlang tests
66+
run: pip install -e python/
6567
- name: Run CI with Maven
6668
run: python ./ci/run_ci.py java --version ${{ matrix.java-version }}
6769
- name: Upload Test Report
@@ -93,6 +95,8 @@ jobs:
9395
python-version: 3.8
9496
- name: Install bazel
9597
run: python ./ci/run_ci.py cpp --install-deps-only
98+
- name: Install pyfory for xlang tests
99+
run: pip install -e python/
96100
- name: Install python dependencies
97101
run: pip install pyarrow==15.0.0 Cython wheel pytest setuptools -U
98102
- name: Run CI with Maven
@@ -190,10 +194,10 @@ jobs:
190194
runs-on: ubuntu-latest
191195
steps:
192196
- uses: actions/checkout@v5
193-
- name: Set up JDK8
197+
- name: Set up JDK11
194198
uses: actions/setup-java@v4
195199
with:
196-
java-version: 8
200+
java-version: 11
197201
distribution: "temurin"
198202
- name: Set up Python 3.8
199203
uses: actions/setup-python@v5
@@ -272,9 +276,14 @@ jobs:
272276
uses: actions/setup-python@v5
273277
with:
274278
python-version: ${{ matrix.python-version }}
275-
- name: Install bazel
279+
- name: Install bazel (Unix)
280+
if: runner.os != 'Windows'
276281
shell: bash
277282
run: python ./ci/run_ci.py cpp --install-deps-only
283+
- name: Install bazel (Windows)
284+
if: runner.os == 'Windows'
285+
shell: bash
286+
run: ./ci/run_ci.sh install_bazel_windows
278287
- name: Run Python CI
279288
shell: bash
280289
run: python ./ci/run_ci.py python
@@ -301,7 +310,7 @@ jobs:
301310
run: python ./ci/run_ci.py cpp --install-deps-only
302311
- name: Install python dependencies
303312
run: pip install pyarrow==15.0.0 cython wheel pytest setuptools -U
304-
- name: Install pyfory
313+
- name: Install pyfory for xlang tests
305314
run: pip install -e python/
306315
- name: Run Golang CI
307316
run: python ./ci/run_ci.py go

ci/run_ci.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ def run_shell_script(command, *args):
8484
cmd = [bash_path, script_path, command]
8585
cmd.extend(args)
8686
logging.info(f"Falling back to shell script with bash: {' '.join(cmd)}")
87-
return subprocess.call(cmd)
87+
sys.exit(subprocess.call(cmd))
8888
else:
8989
logging.error(
9090
"Bash is not available on this Windows system. Cannot run shell script."
@@ -101,7 +101,7 @@ def run_shell_script(command, *args):
101101
cmd = [script_path, command]
102102
cmd.extend(args)
103103
logging.info(f"Falling back to shell script: {' '.join(cmd)}")
104-
return subprocess.call(cmd)
104+
sys.exit(subprocess.call(cmd))
105105

106106

107107
def parse_args():
@@ -237,14 +237,13 @@ def parse_args():
237237
if USE_PYTHON_JAVA:
238238
func(**arg_dict)
239239
else:
240-
241240
if not arg_dict.get("version"):
242241
func(**arg_dict)
243242
return
244243
# Map Python version argument to shell script command
245244
version = arg_dict.get("version", "17")
246245
release = arg_dict.get("release", False)
247-
246+
248247
if release:
249248
logging.info("Release mode requested - using Python implementation")
250249
func(**arg_dict)

ci/run_ci.sh

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ install_jdks() {
141141

142142
graalvm_test() {
143143
cd "$ROOT"/java
144-
mvn -T10 -B --no-transfer-progress clean install -DskipTests
144+
mvn -T10 -B --no-transfer-progress clean install -DskipTests -pl '!:fory-format,!:fory-testsuite'
145145
echo "Start to build graalvm native image"
146146
cd "$ROOT"/integration_tests/graalvm_tests
147147
mvn -DskipTests=true --no-transfer-progress -Pnative package
@@ -231,7 +231,7 @@ case $1 in
231231
echo "Executing fory java tests"
232232
cd "$ROOT/java"
233233
set +e
234-
mvn -T16 --batch-mode --no-transfer-progress test
234+
mvn -T16 --batch-mode --no-transfer-progress test -pl '!:fory-format,!:fory-testsuite'
235235
testcode=$?
236236
if [[ $testcode -ne 0 ]]; then
237237
exit $testcode

ci/tasks/java.py

Lines changed: 35 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717

1818
import logging
1919
import os
20-
import sys
2120
import subprocess
2221
import re
2322
from . import common
@@ -26,7 +25,7 @@
2625
def get_jdk_major_version():
2726
try:
2827
# Run the 'java -version' command
29-
result = subprocess.run(['java', '-version'], capture_output=True, text=True)
28+
result = subprocess.run(["java", "-version"], capture_output=True, text=True)
3029
output = result.stderr # java -version outputs to stderr
3130

3231
# Use regex to find the version string
@@ -37,8 +36,8 @@ def get_jdk_major_version():
3736
version_string = match.group(1)
3837

3938
# Parse the version string
40-
version_parts = version_string.split('.')
41-
if version_parts[0] == '1':
39+
version_parts = version_string.split(".")
40+
if version_parts[0] == "1":
4241
# Java 8 or earlier
4342
return int(version_parts[1])
4443
else:
@@ -81,54 +80,56 @@ def create_toolchains_xml(jdk_mappings):
8180
import os
8281
import xml.etree.ElementTree as ET
8382
from xml.dom import minidom
84-
83+
8584
# Create ~/.m2 directory if it doesn't exist
8685
m2_dir = os.path.expanduser("~/.m2")
8786
os.makedirs(m2_dir, exist_ok=True)
88-
87+
8988
# Create the root element
9089
toolchains = ET.Element("toolchains")
91-
90+
9291
for version, jdk_name in jdk_mappings.items():
9392
toolchain = ET.SubElement(toolchains, "toolchain")
94-
93+
9594
# Set type
9695
type_elem = ET.SubElement(toolchain, "type")
9796
type_elem.text = "jdk"
98-
97+
9998
# Set provides
10099
provides = ET.SubElement(toolchain, "provides")
101100
version_elem = ET.SubElement(provides, "version")
102101
version_elem.text = version
103102
vendor_elem = ET.SubElement(provides, "vendor")
104103
vendor_elem.text = "azul"
105-
104+
106105
# Set configuration
107106
configuration = ET.SubElement(toolchain, "configuration")
108107
jdk_home = ET.SubElement(configuration, "jdkHome")
109108
jdk_home.text = os.path.abspath(os.path.join(common.PROJECT_ROOT_DIR, jdk_name))
110-
109+
111110
# Create pretty XML string
112-
rough_string = ET.tostring(toolchains, 'unicode')
111+
rough_string = ET.tostring(toolchains, "unicode")
113112
reparsed = minidom.parseString(rough_string)
114113
pretty_xml = reparsed.toprettyxml(indent=" ")
115-
114+
116115
# Add proper XML header with encoding
117116
xml_header = '<?xml version="1.0" encoding="UTF8"?>\n'
118-
pretty_xml = xml_header + pretty_xml.split('\n', 1)[1] # Remove the default header and add our custom one
119-
117+
pretty_xml = (
118+
xml_header + pretty_xml.split("\n", 1)[1]
119+
) # Remove the default header and add our custom one
120+
120121
# Write to ~/.m2/toolchains.xml
121122
toolchains_path = os.path.join(m2_dir, "toolchains.xml")
122-
with open(toolchains_path, 'w', encoding='utf-8') as f:
123+
with open(toolchains_path, "w", encoding="utf-8") as f:
123124
f.write(pretty_xml)
124-
125+
125126
logging.info(f"Created toolchains.xml at {toolchains_path}")
126127
logging.info("Toolchains configuration:")
127128
for version, jdk_name in jdk_mappings.items():
128129
jdk_path = os.path.join(common.PROJECT_ROOT_DIR, jdk_name)
129130
logging.info(f" JDK {version}: {jdk_path}")
130131
# print toolchains.xml
131-
with open(toolchains_path, 'r', encoding='utf-8') as f:
132+
with open(toolchains_path, "r", encoding="utf-8") as f:
132133
logging.info(f.read())
133134

134135

@@ -149,7 +150,9 @@ def run_java8():
149150
logging.info("Executing fory java tests with Java 8")
150151
install_jdks()
151152
common.cd_project_subdir("java")
152-
common.exec_cmd("mvn -T16 --batch-mode --no-transfer-progress test -pl '!fory-format'")
153+
common.exec_cmd(
154+
"mvn -T16 --batch-mode --no-transfer-progress test -pl '!:fory-format,!:fory-testsuite'"
155+
)
153156
logging.info("Executing fory java tests succeeds")
154157

155158

@@ -208,7 +211,9 @@ def run_integration_tests():
208211
logging.info("Executing fory integration tests")
209212

210213
common.cd_project_subdir("java")
211-
common.exec_cmd("mvn -T10 -B --no-transfer-progress clean install -DskipTests")
214+
common.exec_cmd(
215+
"mvn -T10 -B --no-transfer-progress clean install -DskipTests -pl '!:fory-format,!:fory-testsuite'"
216+
)
212217

213218
logging.info("benchmark tests")
214219
common.cd_project_subdir("java/benchmark")
@@ -270,7 +275,9 @@ def run_graalvm_test():
270275
logging.info("Start GraalVM tests")
271276

272277
common.cd_project_subdir("java")
273-
common.exec_cmd("mvn -T10 -B --no-transfer-progress clean install -DskipTests")
278+
common.exec_cmd(
279+
"mvn -T10 -B --no-transfer-progress clean install -DskipTests -pl '!:fory-format,!:fory-testsuite'"
280+
)
274281

275282
logging.info("Start to build graalvm native image")
276283
common.cd_project_subdir("integration_tests/graalvm_tests")
@@ -285,17 +292,19 @@ def run_graalvm_test():
285292

286293
def run_release():
287294
"""Release to Maven Central."""
288-
logging.info(f"Starting release to Maven Central with Java")
295+
logging.info("Starting release to Maven Central with Java")
289296
common.cd_project_subdir("java")
290-
297+
291298
# Clean and install without tests first
292299
logging.info("Cleaning and installing dependencies")
293300
common.exec_cmd("mvn -T10 -B --no-transfer-progress clean install -DskipTests")
294-
301+
295302
# Deploy to Maven Central
296303
logging.info("Deploying to Maven Central")
297-
common.exec_cmd("mvn -T10 -B --no-transfer-progress clean deploy -Dgpg.skip -DskipTests -Papache-release")
298-
304+
common.exec_cmd(
305+
"mvn -T10 -B --no-transfer-progress clean deploy -Dgpg.skip -DskipTests -Papache-release"
306+
)
307+
299308
logging.info("Release to Maven Central completed successfully")
300309

301310

integration_tests/graalvm_tests/pom.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,6 @@
173173
<!-- for fast build -->
174174
<!-- <buildArg>-Ob</buildArg> -->
175175
<buildArg>-H:+UnlockExperimentalVMOptions</buildArg>
176-
<buildArg>-H:DynamicProxyConfigurationFiles=src/main/resources/META-INF/native-image/proxy-config.json</buildArg>
177176
</buildArgs>
178177
</configuration>
179178
</plugin>

integration_tests/graalvm_tests/src/main/java/org/apache/fory/graalvm/ObjectStreamExample.java

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -17,42 +17,42 @@
1717
* under the License.
1818
*/
1919

20-
package org.apache.fory.graalvm;
21-
22-
import org.apache.fory.Fory;
20+
package org.apache.fory.graalvm;
2321

2422
import java.util.AbstractMap;
2523
import java.util.Arrays;
2624
import java.util.HashSet;
2725
import java.util.Set;
26+
import org.apache.fory.Fory;
2827

2928
public class ObjectStreamExample extends AbstractMap<Integer, Integer> {
30-
private static final Fory FORY = Fory.builder()
31-
.withName(ObjectStreamExample.class.getName())
32-
.registerGuavaTypes(false)
33-
.build();
29+
private static final Fory FORY =
30+
Fory.builder()
31+
.withName(ObjectStreamExample.class.getName())
32+
.registerGuavaTypes(false)
33+
.build();
3434

3535
static {
36-
FORY.register(ObjectStreamExample.class, true);
37-
FORY.ensureSerializersCompiled();
36+
FORY.register(ObjectStreamExample.class, true);
37+
FORY.ensureSerializersCompiled();
3838
}
3939

4040
final int[] ints = new int[10];
4141

4242
public static void main(String[] args) {
43-
FORY.reset();
44-
byte[] bytes = FORY.serialize(new ObjectStreamExample());
45-
FORY.reset();
46-
ObjectStreamExample o = (ObjectStreamExample) FORY.deserialize(bytes);
47-
System.out.println(Arrays.toString(o.ints));
43+
FORY.reset();
44+
byte[] bytes = FORY.serialize(new ObjectStreamExample());
45+
FORY.reset();
46+
ObjectStreamExample o = (ObjectStreamExample) FORY.deserialize(bytes);
47+
System.out.println(Arrays.toString(o.ints));
4848
}
4949

5050
@Override
5151
public Set<Entry<Integer, Integer>> entrySet() {
52-
HashSet<Entry<Integer, Integer>> set = new HashSet<>();
53-
for (int i = 0; i < ints.length; i++) {
54-
set.add(new AbstractMap.SimpleEntry<>(i, ints[i]));
55-
}
56-
return set;
52+
HashSet<Entry<Integer, Integer>> set = new HashSet<>();
53+
for (int i = 0; i < ints.length; i++) {
54+
set.add(new AbstractMap.SimpleEntry<>(i, ints[i]));
55+
}
56+
return set;
5757
}
58-
}
58+
}

integration_tests/graalvm_tests/src/main/java/org/apache/fory/graalvm/ProxyExample.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ private static Fory createFory() {
5050
.build();
5151
// register and generate serializer code.
5252
fory.register(TestInvocationHandler.class, true);
53+
fory.ensureSerializersCompiled();
5354
return fory;
5455
}
5556

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
[
2+
{
3+
"interfaces": [
4+
"java.util.function.Function"
5+
],
6+
"condition": {
7+
"typeReachable": "org.apache.fory.graalvm.ProxyExample$TestInvocationHandler"
8+
}
9+
}
10+
]

integration_tests/graalvm_tests/src/main/resources/META-INF/native-image/proxy-config.json

Lines changed: 0 additions & 7 deletions
This file was deleted.

0 commit comments

Comments
 (0)