Add bash script to autotest cpython lib #4870

MegasKomnenos · 2023-04-10T11:49:15Z

The following PR contains a bash script to automate the initial testing workflow for updating python library components from cpython.

The workflow includes selecting and updating an individual library component, then running the related test to search for bugs. I realized that this work can be automated and be run for every library components, to screen out ones that fail. I wrote a bash script that does that, and made this PR.

To use, write down your local cpython path in clib_path.txt, and run, bash clib_test_one.sh and check the result in clib_out.txt The script will try to test every component in clib_list.txt, where, clib_test_one.sh : test each component individually clib_test_all.sh : test every components simultaneously

It works by making the code to ignore every tokens that come after the comment symbol

It fails in a weird way and crashes the whole system by gobbling up memory

scripts/clib/readme.txt

scripts/clib/clib_test_all.sh

scripts/clib/clib_list.txt

scripts/clib/clib_path.txt

scripts/clib/readme.txt

To use, add # in front of any line that you want to comment out Warning, it won't work unless # is the first character in a string, not including whitespaces

youknowone · 2023-04-25T06:03:29Z

Thank you! let me test it a bit.

youknowone · 2023-04-25T13:50:29Z

scripts/clib/readme.md

+
+If either of those assumptions are false, then you must provide a correct path when running clib_test.py
+
+The script will try to test every component in clib_list.txt


how did you made clib_list? manually or by script?

I copy-pasted the content of the issue page into a txt file, then used regex to filter out its contents

it should be relatively easy to keep clib_list in sync with the release of Python we target.

scripts/clib/clib_out.txt

scripts/clib/clib_test.py

youknowone · 2023-04-25T13:59:49Z

scripts/clib/clib_test.py

+            message.append(f"No cpython/Lib/test/test_{lib}.py")
+
+        if test_do:
+            result = subprocess.run(["cargo", "run", "-q", f"{RPYTHONPATH}/Lib/test/test_{lib}.py"], stdout=subprocess.PIPE, stderr=subprocess.STDOUT)


using cargo run in loop is bad idea because it will run cargo build every time.

I will use ../../target/release/rustpython here, and call cargo build --release only once before loop

But wouldn't it be necessary to do cargo build each time as we're changing its library files? Or maybe the interpreter functions independent of its library python files and simply references whatever is written there when it needs to? I'm not familiar with the structure of the project and I assumed the former to be true, but maybe it's not.

Huh. I'm seeing an odd change in behavior. I'm comparing the previous result stored in clib_out.txt with the new result that I'm getting from this change, and it seems to be slightly different? Some tests that failed before now are now passing. Odd.

clib_out_new.txt
clib_out.txt

I believe that this command is not working as intended.

result = subprocess.run(
[RUSTEXECPATH, "-q", self.path_tpython_test],
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
env={"RUSTPYTHONPATH": "/home/kjh/20231/rust/RustPython/LibTest"}
)

When I run it with the Lib directory removed, it returns ModuleNotFoundError

youknowone · 2023-04-25T14:02:57Z

Any suggestion about the script name? clib doesn't give good sense what the script does.
We may be able to turn this script to auto-updating trivial cpython library easily.

cc @fanninpm @DimitrisJim

scripts/clib/clib_test.py

youknowone · 2023-04-25T14:16:12Z

scripts/clib/clib_test.py

+
+        if os.path.isfile(f"{RPYTHONPATH}/Lib/{lib}.py"):
+            lib_exist = True
+            os.rename(f"{RPYTHONPATH}/Lib/{lib}.py", f"{RPYTHONPATH}/Lib/{lib}_tmp_mv.py")


Rather than renaming original library files, redefining RUSTPYTHONPATH will be fine.

This is the logic I think.

cp -r Lib LibTest (but in python, not shell)

cp ../cpython/Lib/x LibTest/x

RUSTPYTHONPATH=LibTest rustpython Lib/test/test_...

Sure, that would be easy. But I am not fully grasping why that would be a better design. Is it because messing with library files via script may open room for possible human mistakes?

I've tried to use RUSTPYTHONPATH environment variable to control which library files are referenced by rustpython interpreter but I seem to be doing something wrong, as it's working as if it's still referencing files in Lib, not LibTest. Could you guide me through how I could do what you suggested in python?

This is same as build script principle. Build script shouldn't edit source code but create artifact only under build directory.
By isolating build directory from source code, it is guaranteed to be reproducible regardless current state of the source code. Also by avoiding editing current state, catching a mistake is easier by missing files and no worries to make bugs not to correctly edit/recover original files to both the build script and the source code itself.

Not tried myself yet, but how did you set environment variable? It will look like:

subprocess.run( ["rustpython", "-q", self.path_rustpython_test], stdout=subprocess.PIPE, stderr=subprocess.STDOUT, env={"RUSTPYTHONPATH": "LibTest"}, )

I tried to set environment variable through os.environ. Let me try again with a new method and see how it turns out.

scripts/clib/clib_test.py

Changed code structure to prevent rebuilding cargo for each iteration of the loop Made the script to no longer edit the lib files directly, and instead used RUSTPYTHONPATH to reference a temporary lib folder

DimitrisJim · 2023-05-19T12:27:52Z

scripts/clib/clib_out.txt

@@ -0,0 +1,153 @@
+__future__:  OK  


So this is typically the output of running clib_test.py, right?

DimitrisJim · 2023-05-19T12:30:17Z

do we have any updates on the state of this PR? It's been sorta stale for 3 weeks now and we should try and get your work through soon if possible.

youknowone · 2023-05-19T14:45:23Z

My bad, I forgot about this after getting in the parser rally.
The whole idea is awesome. I need to clean up its interface more fit in our other scripts and naming so on.

youknowone · 2023-05-19T14:46:15Z

@DimitrisJim do you have a good name suggestion instead of clib? it is not a C something.

DimitrisJim · 2023-05-19T15:10:00Z

ah, sure, I can pick this up and get it through with @MegasKomnenos. Parser is definitely a more pressing issue so you can give as much attention there as needed.

a good name suggestion instead of clib?

Yeah, clib is not the best too imho. I'd be fine with autotest as the title uses, the README should explain any additional information needed.

MegasKomnenos added 7 commits April 8, 2023 22:53

Add comment functionality to script

6d9ff69

It works by making the code to ignore every tokens that come after the comment symbol

Comment out signal.py

0c87296

It fails in a weird way and crashes the whole system by gobbling up memory

Move clib scripts to scripts/clib

2ae27cc

Use echo to improve tokenization

38e6597

Script output

4eae45e

Add readme

6702516

fanninpm reviewed Apr 10, 2023

View reviewed changes

scripts/clib/readme.txt Outdated Show resolved Hide resolved

youknowone reviewed Apr 10, 2023

View reviewed changes

scripts/clib/clib_test_all.sh Outdated Show resolved Hide resolved

fanninpm reviewed Apr 10, 2023

View reviewed changes

scripts/clib/clib_list.txt Outdated Show resolved Hide resolved

fanninpm reviewed Apr 10, 2023

View reviewed changes

scripts/clib/clib_path.txt Outdated Show resolved Hide resolved

DimitrisJim reviewed Apr 10, 2023

View reviewed changes

scripts/clib/readme.txt Outdated Show resolved Hide resolved

MegasKomnenos added 2 commits April 11, 2023 09:48

Apply feedback on readme

9b6a250

Change how comment works in clib_list.txt

3e69dd6

To use, add # in front of any line that you want to comment out Warning, it won't work unless # is the first character in a string, not including whitespaces

youknowone added the skip:ci Skip running the ci label Apr 20, 2023

MegasKomnenos added 4 commits April 25, 2023 10:55

Change script to be in python

8df30a5

Make script to use append for saving output in clib_out.txt

1034109

Fix bug where stderr was not being captured

2dadaac

Restore output

1d08c65

Ran the new python script

b2a617c

youknowone requested changes Apr 25, 2023

View reviewed changes

youknowone reviewed Apr 25, 2023

View reviewed changes

MegasKomnenos added 2 commits April 26, 2023 12:38

Apply feedback

b8dbdd2

Applied feedback

4e5cd67

Changed code structure to prevent rebuilding cargo for each iteration of the loop Made the script to no longer edit the lib files directly, and instead used RUSTPYTHONPATH to reference a temporary lib folder

DimitrisJim reviewed May 19, 2023

View reviewed changes

scripts/clib/clib_out.txt

@@ -0,0 +1,153 @@

__future__: OK

Copy link

Member

DimitrisJim May 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is typically the output of running clib_test.py, right?

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add bash script to autotest cpython lib #4870

Add bash script to autotest cpython lib #4870

MegasKomnenos commented Apr 10, 2023

youknowone commented Apr 25, 2023

youknowone Apr 25, 2023

MegasKomnenos Apr 26, 2023

DimitrisJim May 19, 2023

youknowone Apr 25, 2023 •

edited

MegasKomnenos Apr 26, 2023

MegasKomnenos Apr 27, 2023

MegasKomnenos Apr 27, 2023

MegasKomnenos May 1, 2023

youknowone commented Apr 25, 2023

youknowone Apr 25, 2023

MegasKomnenos Apr 25, 2023

MegasKomnenos Apr 26, 2023

youknowone Apr 27, 2023

MegasKomnenos Apr 27, 2023

DimitrisJim May 19, 2023

DimitrisJim commented May 19, 2023

youknowone commented May 19, 2023

youknowone commented May 19, 2023

DimitrisJim commented May 19, 2023


		If either of those assumptions are false, then you must provide a correct path when running clib_test.py

		The script will try to test every component in clib_list.txt

Add bash script to autotest cpython lib #4870

Are you sure you want to change the base?

Add bash script to autotest cpython lib #4870

Conversation

MegasKomnenos commented Apr 10, 2023

youknowone commented Apr 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowone Apr 25, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youknowone commented Apr 25, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DimitrisJim commented May 19, 2023

youknowone commented May 19, 2023

youknowone commented May 19, 2023

DimitrisJim commented May 19, 2023

youknowone Apr 25, 2023 •

edited