Created attachment 34246 [details] dstar_OperandResolver_refactor.patch Non functionality changing refactor of DStarRunner.java to not use custom implementations to resolve references or coerce to string, but use the OperandResolver static methods instead. Tests still all pass. Also removes an unused method.
With this patch applied to trunk, but not your other patch, the unit tests fail with: Testcase: processFunctionRow[strange types as headers - wrong number] took 0.001 sec Caused an ERROR Error evaluating cell DGet!B98 org.apache.poi.ss.formula.eval.NotImplementedException: Error evaluating cell DGet!B98 at org.apache.poi.ss.formula.WorkbookEvaluator.addExceptionInfo(WorkbookEvaluator.java:386) at org.apache.poi.ss.formula.WorkbookEvaluator.evaluateAny(WorkbookEvaluator.java:327) at org.apache.poi.ss.formula.WorkbookEvaluator.evaluate(WorkbookEvaluator.java:259) at org.apache.poi.hssf.usermodel.HSSFFormulaEvaluator.evaluateFormulaCellValue(HSSFFormulaEvaluator.java:200) at org.apache.poi.ss.formula.BaseFormulaEvaluator.evaluate(BaseFormulaEvaluator.java:101) at org.apache.poi.ss.formula.functions.BaseTestFunctionsFromSpreadsheet.processFunctionRow(BaseTestFunctionsFromSpreadsheet.java:151) Caused by: org.apache.poi.ss.formula.eval.NotImplementedException: D* function with formula conditions at org.apache.poi.ss.formula.functions.DStarRunner.fullfillsConditions(DStarRunner.java:227) at org.apache.poi.ss.formula.functions.DStarRunner.evaluate(DStarRunner.java:102) at org.apache.poi.ss.formula.functions.DStarRunner.evaluate(DStarRunner.java:55) at org.apache.poi.ss.formula.OperationEvaluatorFactory.evaluate(OperationEvaluatorFactory.java:132) at org.apache.poi.ss.formula.WorkbookEvaluator.evaluateFormula(WorkbookEvaluator.java:550) at org.apache.poi.ss.formula.WorkbookEvaluator.evaluateAny(WorkbookEvaluator.java:317) Do we need to get your other patch fixed and applied first? Or do we need to fix the current tests for this patch?
(In reply to Nick Burch from comment #1) > With this patch applied to trunk, but not your other patch, the unit tests > fail with: > > ... > > Do we need to get your other patch fixed and applied first? Or do we need to > fix the current tests for this patch? I messed up a bit. Sorry for that. Both patches together work fine. There is a fix relating to error values in the D* database headers in here I overlooked while preparing the patches. Because of that the new DGet.xls (which tests that behavior) fails without this patch. Also the DGet.xls has a false-positive test now <skip>ed which this patch relies on. Easiest way forward: Just apply both patches and the DGet.xls. Alternative: I prepare another intermediate DGet.xls so one can apply the patches one after the other. What do you think?
Together is fine.
Patch applied in r1760717, thanks!