Refactor to support BoxLang alongside ACF,Lucee#450
Conversation
|
Thanks for working on this, Chase. My aim was to try and support Boxlang when it first came out as it lacked spreadsheet support at the time. But I ended up abandoning the effort because 1) there were too many failing tests; 2) the tests ran extremely slowly on Boxlang for some reason (frequently timing out); and 3) Ortus eventually added their own spreadsheet module (which looks pretty decent). I hadn't realised that null support (NS) is the default in Boxlang. That might explain some of the issues. People have requested NS support previously but I've hesitated due to the complexity of having to manage 2 different modes of behaviour for each engine. When I get chance I'll take a look through your changes and hopefully revisit both issues. The Lucee 6/7 tests you mention failing in the current branch are probably caused by timezone issues. They pass for me in the UK, but I guess not where you are. Obviously needs addressing. Cheers again for your contribution. Hopefully I'll get back to you soon. |
|
Awesome! Let me know if there is anything you want me to change on my end. I made a few changes to the Github workflow, I was trying to get all the engines to run simultaneously locally using act (https://github.com/nektos/act) but couldn't get reliable results. Ortus offers a cf-compat module to make boxlang 'behave' like the other cf engines - but this pr allows boxlang to run this package without needing the cf-compat module (which is just a temporary band aid until you actually migrate) |
|
@lanechase34 I've merged your PR and it seems to work really well locally. Boxlang runs the test suite faster than the last time I tried, although for me it's still much slower than Lucee/ACF. I managed to fix the write() password encryption issue, so the only test failing in Boxlang was "can read the csv from a text file with an .xls extension". It seems it's the opposite of what you said: Boxlang's FileGetMimeType() must be looking at the extension, instead of the file content/headers. As it's hardly a critical capability (just don't rename your CSV files as .xls!) I've set that test to be skipped in Boxlang. The only thing I've dropped is the "/testbox" mapping in the test suite. That would require testbox to be installed in the project root, whereas I think the location should be configurable to the environment (I like to use one installation across multiple projects). I'm no expert on Github Actions and haven't looked at your changes there. If you think the workflow needs improving, please go ahead. |
Changes:
Results:
2 failing tests in BoxLang still - not sure if they are resolvable. The FileGetMimeType() seems to always look at the content rather than the extension in BoxLang. I am not familiar with a password protected xlsx file - I haven't really looked into that test too deep.
-- can read the csv from a text file with an .xls extension (12 ms)
-- Can write an XLSX file encrypted with a password (89 ms) - The supplied POIFSFileSystem does not contain a BIFF8 'Workbook' entry. Is it really an excel file? Had: []
2 failing tests in Lucee6, Lucee7 - these were present before the changes I made were done.
-- Sets the specified cell to the specified date value even if the Lucee timezone doesn't match the system (13 ms) - Expected [42106] but received [42105.8]
-- allows the format of date and time values to be customised (11 ms) - Expected [2019-04-12] but received [2019-04-11]
ACF2025 all tests pass.